DO NOT MERGE: feat(ledger): upgrade ledger 8 to 8.2.0-rc.1#1869
DO NOT MERGE: feat(ledger): upgrade ledger 8 to 8.2.0-rc.1#1869gilescope wants to merge 4 commits into
Conversation
Signed-off-by: Giles Cope <gilescope@gmail.com>
gilescope
added
skip-changes-check-issue
ai-assisted
This comment has been minimized.
This comment has been minimized.
Signed-off-by: Giles Cope <gilescope@gmail.com>
|
Now that's green, let's add in being able to sync with a ledger8 genesis blockchain... |
![jina-simulation[bot]](https://avatars.githubusercontent.com/u/97225700?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd43e007d6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # like midnight-storage-core 1.2.0 into two source-incompatible copies (E0277: trait `DB` not | ||
| # satisfied). Consequently L8 no longer shares coin-structure/base-crypto/transient-crypto/ | ||
| # serialize with L7 — each gets a dedicated `-ledger-8` entry pinned to the tag. | ||
| mn-ledger-8 = { git = "https://github.com/midnightntwrk/midnight-ledger", tag = "ledger-8.2.0-rc.1", package = "midnight-ledger" } |
There was a problem hiding this comment.
This bump brings Ledger 8 to the 8.2 split-phase API, but the version-specific validation shim is still on the old path: ledger/src/versions/guaranteed_validation/ledger_8.rs:16-35 says Ledger 8 has no apply_guaranteed_only and dry-runs the full state.apply(...). When a node/runtime is still using the Ledger8 bridge, mempool and pre-dispatch validation of a transaction with an expensive or failing fallible segment will still execute that fallible work even though acceptance only depends on the guaranteed segment, undermining the purpose of the 8.2 upgrade and leaving a avoidable validation-cost/DoS path. Please switch the Ledger8 shim to the new apply_guaranteed_only API as part of this dependency bump.
Useful? React with 👍 / 👎.
Signed-off-by: Giles Cope <gilescope@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e99718e5b9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // L8 (git-8.2 storage-core) and L9 (crates.io storage-core) no longer share a storage arena, | ||
| // so the old in-place arena-key reinterpret is invalid. Move the value across by serializing in | ||
| // the L8 format and deserializing in L9, then allocate it into L9 storage. | ||
| let value: T2 = old_to_new_ser(&*t1)?; |
There was a problem hiding this comment.
When fork_context_8_to_9 calls this for the ledger state, this now serializes with ledger-8's tagged format and deserializes with ledger-9's tagged format. The state tags are intentionally different (ledger-state[v13] vs ledger-state[v18], as documented in ledger/src/versions/common/storage.rs), so any toolkit path that forks an L8 context to L9 will return a deserialization error before migration instead of producing the new context; this replacement for the old arena-key copy needs an untagged or otherwise version-aware conversion for types whose tag changed.
Useful? React with 👍 / 👎.
gilescope
changed the title
1 participant
UPDATE: We'd prefer to move HF onwards and get to ledger 9 rather than upgrade 8.1.
Overview
Bumps the ledger-8 stack from 8.1.0 to
midnight-ledger8.2.0-rc.1 (picking upapply_guaranteed_only, infalliblepost_block_update, split-phase execution with deferred events, and the segment-type refactor), keeping ledger 9 as-is.8.2.0-rc.1 is not published to crates.io, so the ledger-8 crates are sourced from the
ledger-8.2.0-rc.1git tag. Because the tag carries path deps, ledger 8 no longer shares companion crates with ledger 7 — each companion gets a dedicated-ledger-8workspace entry pinned to the tag (a git-sourced 2.0.1 is a distinct type family from the crates.io 2.0.1, hence the separate conversion impl set inbuilders/mod.rs).Notable mechanics:
DefaultHasherinstead of hardcodedsha2::Sha256, so it tracks the git-8.2 storage-core's sha2 0.11 bump automatically.SerializedTxis generic over theBlockContextversion (SerializedTx<C = ledger_9::BlockContext>);extract_tx_with_context_ledger_8deserializes directly into an L8 context (the JSON wire shape is version-agnostic viaSerdeBlockContext), replacing the field-by-field rebuild.Follow up PRs:
🗹 TODO before merging
📌 Submission Checklist
git commit -s) for the DCOchanges/node/changed/ledger-8-2-upgrade.md🧪 Testing Evidence
cargo test -p midnight-node-ledger -p pallet-midnight— 93 + 28ledger_8::common::apitests green, includingshould_validate_transaction/should_apply_transactionwhich parse the realDEPLOY_TXfixture through the new directSerializedTx<ledger_8::BlockContext>deserialization and apply it to genesis state.cargo clippyclean acrossmidnight-node-ledger-helpers(--features can-panic) andmidnight-node-toolkit.Additional tests are provided (if possible)
🔱 Fork Strategy
Assisted-by: Claude:claude-fable-5