-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Substrate.io Migration] - Polkadot Protocol > Protocol Components > Transactions > Transactions Formats #80
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall good, just a few suggestions for the content
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall lgtm, the back linking to the psdk is especially important here
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
|
||
Then, some additional data that's not part of what gets signed is required, which includes: | ||
|
||
- The spec version and the transaction version ensure that the transaction is submitted to a [compatible runtime](TODO:update-path){target=\_blank} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you link in compatible runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One of the bader's suggestions were linking this part with "Upgrade your runtime"
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
Co-authored-by: 0xLucca <[email protected]>
Pushed a few formatting changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor formatting stuff. But I also left a comment about linking to specific lines. I didn't actually check to see if we could link to another part of the Rust docs, but can you please look into it and see if we can do that instead of specific file lines?
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
polkadot-protocol/protocol-components/transactions/transactions-formats.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Erin Shaben <[email protected]>
Just requesting re-review for pending approvals |
This PR aims to migrate this page of the substrate tutorials: https://docs.substrate.io/learn/transaction-formats/
Disclaimer: I did not migrate this section: https://docs.substrate.io/reference/transaction-format/#:~:text=Polkadot%20JS%20Apps%20example%3A
Since I believe that would fit better in the tutorials section, maybe we can add a reference to that part in the future. However, I'm open to discuss about it :)