Skip to content
This repository was archived by the owner on Apr 8, 2022. It is now read-only.

Updated docs for node start-up, tokens, and transactions#120

Merged
royster57 merged 1 commit intostagingfrom
roy/docs_no_wallet_fix
Nov 23, 2021
Merged

Updated docs for node start-up, tokens, and transactions#120
royster57 merged 1 commit intostagingfrom
roy/docs_no_wallet_fix

Conversation

@royster57
Copy link

No description provided.

@royster57 royster57 force-pushed the roy/docs_no_wallet_fix branch 2 times, most recently from 33ca63d to 32b5652 Compare November 15, 2021 08:06
--validator \
--rpc-external \
--rpc-methods Unsafe \
--chain=Testnet1Spec.json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never used the --rpc-external and --rpc-methods Unsafe options so at least for basic usage, these are not necessary. The question is whether users should pass them by default. Ideally, the command to run the node should be as simple as possible.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, the rpc-external flag binds to all network interfaces with IP 0.0.0.0, meaning that anyone can connect to the node from outside the network. And rpc-methods Unsafe, basically allows anyone connecting to the RPC to be an owner of the node, and hence, expose keys. Meaning: We're basically exposing everything to everyone and allowing anyone connecting to RPC to do whatever they want... what's the logic behind this?! Did we try not adding them and had problems?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the command to run as I understood it from Amin.
@zorvan What do you think of Lukas's and Sam's comments?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are right. Latest version of cli switches for user and production id reaped off from those.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zorvan so what is the correct command? Should I simply remove --rpc-external and --rpc-methods Unsafe?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in every issue in programming, that depends on what you want to do. The right solution, in my opinion, is to remove these and tell the user that they will be binding to localhost, hence, no connections from the outside will be possible and explain that this is a safety procedure and using tunnels is usually better than opening to the outside, and then explain that unsafe methods will not work unless the user enables them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And let me explain that hacks with externally accessible RPC happened before:

https://www.coindaily.co/2801-2/

docs/tokens.md Outdated

A transaction involving transaction output carries a (possibly empty) `data` field specifying the purpose of the transaction.

**TOOD when is there no data field?**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be here TODO instead of TOOD?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup

docs/tokens.md Outdated

**TOOD when is there no data field?**

The data field can be any of the fol
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

following?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

docs/tokens.md Outdated

**TODO is the sentence below true? In the example, I don't see any data relating to the token in the input.**

When issuing a new token, we specify the data for creating a new token in the transaction input, where the `token_id` is a hash of the inputs.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When issuing a new token, we specify the data for creating a new token in the transaction output. After that, a wallet should calculate the token_id that is a hash of the first input of the issuance transaction. And use this token_id is able to send tokens to anyone.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that clarifies things! I will modify the explanation.

docs/tokens.md Outdated

When issuing a new token, we specify the data for creating a new token in the transaction input, where the `token_id` is a hash of the inputs.

**TODO explain the remaining fields (token_ticker, metadata_URI)**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

token_ticker is a short name of the token. No longer than 5 symbols.

metadata_URI is a web link to a JSON file where we can store additional information for the token: #67 Over here you can find the paragraph: Format of data located by reference metadata_URI

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sinitcin Which characters can be used in the token ticker? Are numbers, special characters allowed?

docs/tokens.md Outdated

**TODO explain the remaining fields (token_ticker, metadata_URI)**

**TODO the token issuance struct shown here has a token_id which does not appear in the example. In staging, this is commented out. Why?**
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During the review process, Sam tell me that I have to remove token_id in TokenIssuanceV1 because it's a calculatable field from an of the first input. I will clear comments.

@royster57 royster57 force-pushed the roy/docs_no_wallet_fix branch 5 times, most recently from b8767ec to 838dc5d Compare November 22, 2021 07:43
@royster57 royster57 force-pushed the roy/docs_no_wallet_fix branch from 838dc5d to 5cc65bb Compare November 22, 2021 09:37
@royster57 royster57 merged commit 7b34458 into staging Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants