search

paritytech / subxt

Interact with Substrate based nodes in Rust or WebAssembly
Other
391 stars 236 forks source link

Add a basic version of the CheckMetadataHash signed extension #1590

Closed jsdw closed 1 month ago

jsdw commented 1 month ago

This version doesn't try to hash the metadata and provide a hash at the moment, and just tells the node that there will be no hash via a 0 byte in the tx payload.

I tested this by compiling polkadot from https://github.com/paritytech/polkadot-sdk/pull/4274 and running cargo run --example tx_basic with and without the new extension. It failed without and worked with.

Close #1605

FlorianFranzen commented 3 weeks ago

So this break the PolkadotConfig so it only works on testchains while leaving a cryptic "Invalid Transaction: Transaction has a bad signature" on submission for all others

Barely mentioned in the change log that this could happen, e.g. even the staking miner included this release and has that issue now.

Not to mention that all 0.36 user will upgrade to this as well as soon as they run into any issues...

niklasad1 commented 3 weeks ago

@FlorianFranzen do you mind filing an issue in the staking-miner repo?

I wonder why it's just works on "test chains" ^^

jsdw commented 3 weeks ago

So this break the PolkadotConfig so it only works on testchains while leaving a cryptic "Invalid Transaction: Transaction has a bad signature" on submission for all others

Could you provide a way for us to reproduce this, please?

Adding the new extension to PolkadotConfig/SubstrateConfig shouldn't have broken anything; the extension is only made use of if the node's metadata is asking for it (which would have previously broken Subxt anyway, since it didn't exist), and the new extension was (and is currently being tested each night) and we haven't seen any issues around it.

FlorianFranzen commented 3 weeks ago

@jsdw I looked at in detail and tried to create a trivial example without success, so it must be particular to our use-case. The last time I looked at this was also before the runtime update on Polkasama, so that might also play a role is guiding me to believing it is the config that caused the issue.

The issue also appeared in the miner, but it seems unrelated. I currently lack the time to look into this further. Sorry for the false alert. I will post an update should I uncover anything of value.