Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

bump transaction_version#4955

Merged
coderobe merged 3 commits intorelease-v0.9.17from
coderobe/txver
Feb 22, 2022
Merged

bump transaction_version#4955
coderobe merged 3 commits intorelease-v0.9.17from
coderobe/txver

Conversation

@coderobe
Copy link
Copy Markdown
Contributor

@coderobe coderobe commented Feb 21, 2022

there have been storage deletions and parameter changes, this bumps the transaction version accordingly

@coderobe coderobe added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit. labels Feb 21, 2022
@coderobe
Copy link
Copy Markdown
Contributor Author

coderobe commented Feb 21, 2022

i'm unsure if this is necessary
if it turns out to be, i will also pick these commits in a pr against master

@coderobe coderobe added the C1-low PR touches the given topic and has a low impact on builders. label Feb 21, 2022
@EgorPopelyaev
Copy link
Copy Markdown
Contributor

EgorPopelyaev commented Feb 21, 2022

@coderobe Mara, I think westend also needs to be bumped, there are two new calls were added

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Feb 21, 2022

Adding calls or doing anything storage related doesn't requires a transaction version bump. Only if a call has new parameters or a call is being removed requires the bump of the transaction version.

@bkchr
Copy link
Copy Markdown
Member

bkchr commented Feb 21, 2022

[forceCleanHrmp] idx: 3 (args: 1 -> 3)
                                             (u32) -> (u32, u32, u32)
                      [forceProcessHrmpOpen] idx: 4 (args: 0 -> 1)
                                             () -> (u32)
                     [forceProcessHrmpClose] idx: 5 (args: 0 -> 1)
                                             () -> (u32)
                     [hrmpCancelOpenRequest] idx: 6 (args: 1 -> 2)
                                             (PolkadotParachainPrimitivesHrmpChannelId) -> (PolkadotParachainPrimitivesHrmpChannelId, u32)

Okay, I had overseen these ones..

The first two are root calls and the last one is only callable by Parachains.

They require that the transaction version is bumped, but if you are pragmatic it probably creates more hassle than it benefits anyone. There is no root account, so no one can use a hardware wallet to create these transactions.

@joepetrowski
Copy link
Copy Markdown
Contributor

After speaking with some wallet teams, they prefer to bump transaction_version even if it does not affect calls supported in their wallets. Namely for changes to:

  • Pallet order
  • Call order
  • Function signature (number and types of args)

that would prevent accurate decoding of extrinsics from previous versions.

@coderobe
Copy link
Copy Markdown
Contributor Author

bot merge

@paritytech-processbot
Copy link
Copy Markdown

Error: Checks failed for 7086ac2

@coderobe coderobe merged commit de0ecd4 into release-v0.9.17 Feb 22, 2022
@coderobe coderobe deleted the coderobe/txver branch February 22, 2022 08:49
@coderobe coderobe mentioned this pull request Apr 21, 2022
29 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants