Skip to content

Replace sp_io::trie host functions with sp_trie equivalents#344

Closed
x3c41a wants to merge 1 commit into
mainfrom
fix/chopsticks-trie-host-functions
Closed

Replace sp_io::trie host functions with sp_trie equivalents#344
x3c41a wants to merge 1 commit into
mainfrom
fix/chopsticks-trie-host-functions

Conversation

@x3c41a
Copy link
Copy Markdown
Contributor

@x3c41a x3c41a commented Mar 20, 2026

Summary

  • Replace sp_io::trie::blake2_256_ordered_root and sp_io::trie::blake2_256_verify_proof host function calls in pallet-transaction-storage with direct sp_trie pure-Rust equivalents
  • This makes the runtime compatible with Chopsticks, which doesn't implement ext_trie_blake2_256_verify_proof_version_2

Closes #341

@x3c41a x3c41a requested a review from karolk91 March 20, 2026 10:41
@x3c41a x3c41a force-pushed the fix/chopsticks-trie-host-functions branch from 7aac4d5 to 79023f5 Compare March 20, 2026 10:48
@x3c41a
Copy link
Copy Markdown
Contributor Author

x3c41a commented Mar 20, 2026

You need to use a more recent block for tests, the one that you mentioned in the issue description uses an old runtime and causes weird issues. I checked with a recent block (688910) and it worked

@x3c41a x3c41a force-pushed the fix/chopsticks-trie-host-functions branch from 79023f5 to a7f6921 Compare March 20, 2026 10:59
Comment thread pallets/transaction-storage/src/lib.rs Outdated
@karolk91
Copy link
Copy Markdown
Collaborator

You need to use a more recent block for tests, the one that you mentioned in the issue description uses an old runtime and causes weird issues. I checked with a recent block (688910) and it worked

Not sure I understand this one, could you provide the error log ?

@x3c41a
Copy link
Copy Markdown
Contributor Author

x3c41a commented Mar 20, 2026

You need to use a more recent block for tests, the one that you mentioned in the issue description uses an old runtime and causes weird issues. I checked with a recent block (688910) and it worked

Not sure I understand this one, could you provide the error log ?

With the previous block number (the one you have in the issue) I am getting:

      runtime::frame-support  DEBUG: [1] ✅ no migration for BridgePolkadotParachains
      runtime::frame-support  DEBUG: [1] ✅ no migration for BridgePolkadotMessages
      runtime::frame-support  DEBUG: [1] ✅ no migration for Sudo
      runtime::frame-support  DEBUG: [1] ✅ no migration for Proxy
          runtime::executive  DEBUG: [1] [637328]: Block initialization weight consumption: Weight { ref_time: 2312304000, proof_size: 0 }
                     runtime  ERROR: [1] panicked at /Users/ndk/parity/polkadot-bulletin-chain/runtimes/bulletin-polkadot/src/lib.rs:1022:1:
Bad input data provided to apply_extrinsic: Codec error. Nothing bad happened: someone sent an invalid transaction to the node.
Error: wasm `unreachable` instruction executed
    at Object.handler (/Users/ndk/.npm/_npx/81ad9c881cb83600/node_modules/@acala-network/chopsticks/dist/cjs/plugins/run-block/cli.js:67:19)
    ```
    even after my fix.
    
    The error is unrelated and if you just update block to a newer one it will be gone

@karolk91
Copy link
Copy Markdown
Collaborator

karolk91 commented Mar 20, 2026

@x3c41a, this change seem to move the calculations from the native/host execution to the WASM code - avoiding the host calls that are not implemented by smoldot.

Im not sure about all the implications yet but lets start with re-doing the benchmarks / re-generate weights (at least for affected calls like (check_proof, store, renew) -> we will see how/if that affects in any meaningful way

and compare WASM sizes if something doesn't blow up there

@karolk91
Copy link
Copy Markdown
Collaborator

I will also do some stress testing using this change, WASM memory is quite limited - I'm not sure if this change will not increase the WASM memory usage and affect the overall throuput or cause some OOM issues. But I'm just speculating - I dont have experience with this. But lets also wait for my stress testing results

@bkontur
Copy link
Copy Markdown
Collaborator

bkontur commented Mar 20, 2026

@x3c41a, this change seem to move the calculations from the native/host execution to the WASM code - avoiding the host calls that are not implemented by smoldot.

Hmm, let's be very sure about merging this change. I don't know what are the trade-offs here, but what about just adding those host functions to the smoldot like we did here: paritytech/smoldot-archive#2189 ?

@karolk91
Copy link
Copy Markdown
Collaborator

@x3c41a, this change seem to move the calculations from the native/host execution to the WASM code - avoiding the host calls that are not implemented by smoldot.

Hmm, let's be very sure about merging this change. I don't know what are the trade-offs here, but what about just adding those host functions to the smoldot like we did here: smol-dot/smoldot#2189 ?

I agree that would probably be a more safe approach. Definitely worth to explore and compare both

@x3c41a x3c41a force-pushed the fix/chopsticks-trie-host-functions branch from 35c6b8f to 2bf475e Compare March 20, 2026 12:24
@x3c41a
Copy link
Copy Markdown
Contributor Author

x3c41a commented Mar 20, 2026

Yeah, good calls, guys! I was feeling a danger with this PR (also remembered that smoldot PR that @bkontur mentioned) but switched into RPC nodes design doc and then got caught by the UI changes.

Lesson learned: do not take new/risky things on Fridays

@x3c41a x3c41a closed this Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chopsticks doesn't work for blocks where proofs are verified

3 participants