BEEFY session keys and worker - #2084
Conversation
Signed-off-by: microproofs <kwhitemsg@gmail.com>
Signed-off-by: microproofs <kwhitemsg@gmail.com>
…rsion Signed-off-by: microproofs <kwhitemsg@gmail.com>
Fix committee_membership. Fix genesis block building. Read beefy key from its own seed. Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
![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: 9596e72509
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 1, | ||
| 2, |
There was a problem hiding this comment.
When upgrading a chain that has already run the immediate predecessor, this migration is skipped: that runtime wires pallet_session_validator_management::migrations::v2::V1ToV2Migration, which writes storage version 2 while the committee and session entries still use the two-key LegacySessionKeys encoding. Because this VersionedMigration only executes at version 1, the new runtime subsequently decodes those bytes as three-key session data without translating them, causing committee reads to fail and preventing the next session rotation from installing the expected authorities. Use a distinct key-shape storage version or explicitly support both predecessor versions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The predecessor of the release this code is intended for does not contain V1ToV2Migration.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1adb3eb930
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
|
Hi @LGLO — we’re working on Ura Finance and have been testing a trust-minimized Midnight → Cardano reverse-bridge path. We reached a very specific protocol boundary and wanted to check whether #2084 is the upstream piece we should align with rather than inventing anything parallel. What we have established on the application side:
Where our current Preprod research stops is the cross-chain proof boundary. GRANDPA finality is available, but we could not derive a complete, independently verifiable chain from a pinned trust root through authority-set evolution + finalized header + withdrawal transaction/state inclusion without trusting an RPC/indexer. We then found the existing Midnight bridge-proof lineage (#222, #233, #235) and the current Could you clarify four things?
We are reproducing the canonical BEEFY/MMR/RelayChainProof stack locally against the #2084 line now. We’re happy to contribute an external verifier harness, adversarial/negative vectors, RPC conformance tests, and Cardano-side verification work once the intended proof interface is confirmed. Our design constraint is deliberate: RPC/indexer/relayer may transport proof bytes, but none should become a validity authority. We’d rather stop than add an Ura signer/oracle/federation workaround. If there is a Stage-2 bridge technical owner/spec we should coordinate with, a pointer would be very helpful. @b-yap @justinfrevert, tagging because your earlier authority-proof/Cardano-encoding work appears directly relevant. |
|
Follow-up with concrete reproduction results from the exact #2084 pin ( So Midnight's canonical BEEFY/MMR/authority-proof/RelayChainProof stack reproduces locally end-to-end for us. We also verified a full authority-set transition and independent MMR verification. No Ura crypto or authority was added. The remaining public-network delta from our Preprod tests is now narrow: This changes our question from “is the architecture missing?” to “what is the intended deployment/interface path for the already-existing canonical proof stack?” We are deliberately keeping the reverse bridge blocked rather than falling back to an indexer/oracle/signer. If helpful, we can share the reproduction matrix and contribute an external verifier/conformance suite around:
The one remaining application-specific step on our side is to bind our atomic Zswap withdrawal commitment into the authenticated state-inclusion path; we are not asking Midnight to solve that application logic for us. Would appreciate guidance on whether #2084 + the existing |
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09e6998023
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let grandpa = keys.find(GRANDPA)?; | ||
| let grandpa = ed25519::Public::from_raw(grandpa.try_into().ok()?); | ||
| Some(Self { aura: aura.into(), grandpa: grandpa.into() }) | ||
| let beefy = keys.find(BEEFY).or_else(|| keys.find(CROSS_CHAIN))?; |
There was a problem hiding this comment.
For V0/db-sync and local-mock inputs, CandidateKeys contains only AURA and GRANDPA; the usable ECDSA identity is stored separately in RegistrationData.cross_chain_pub_key or PermissionedCandidateData.sidechain_public_key (for example, primitives/mainchain-follower/src/data_source/candidates_data_source/mod.rs:272 and partner-chains/toolkit/data-sources/mock/src/candidate.rs:115). Consequently this ? rejects every such candidate, authority selection returns None, and the pallet repeatedly retains the queued committee instead of applying registrations or permissioned-candidate changes. Fresh evidence in this revision is that the new CROSS_CHAIN lookup still searches the key list where those sources do not place the identity; derive the fallback from the candidate's separate ECDSA identity or add it to CandidateKeys before validation.
AGENTS.md reference: AGENTS.md:L69-L69
Useful? React with 👍 / 👎.
| | AURA seed | `AURA_SEED_FILE=/path/to/seed` | - | Path to AURA consensus seed file | | ||
| | GRANDPA seed | `GRANDPA_SEED_FILE=/path/to/seed` | - | Path to GRANDPA finality seed file | | ||
| | Cross-chain seed | `CROSS_CHAIN_SEED_FILE=/path/to/seed` | - | Path to cross-chain seed file | | ||
| | BEEFY seed | `BEEFY_SEED_FILE=/path/to/seed` | - | Path to BEEFY seed file (ECDSA, like cross-chain) | |
There was a problem hiding this comment.
The single-node example immediately below this new option still supplies only AURA, GRANDPA, and cross-chain seeds and does not use --alice; node/src/command.rs inserts a beef keystore entry only when BEEFY_SEED_FILE is set. Because the dev authority's registered BEEFY public key has no matching secret under the BEEFY key type, this advertised one-validator setup produces blocks but cannot produce BEEFY signatures. Fresh evidence is that the multi-node examples were updated with the new variable while the single-node command at line 308 remains unchanged.
AGENTS.md reference: AGENTS.md:L69-L69
Useful? React with 👍 / 👎.
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
|
Thanks for the detailed note @alba-press — this PR is about enabling BEEFY in our node line, so I’ll briefly describe what we’re actually doing. Work done as part of the item is one of the preliminary items of the bigger midnightntwrk/midnight-improvement-proposals#262 that is already in progress of refinement (please do not consider above document as final). As part of #2084, we plan to make BEEFY operational in a way that it will be able to provide signed commitments and follow canonical chain finalized by GRANDPA. The exact shape of the commitments and what will be in MMR proofs is still discussed, I've hope more details we will have closer to the next quarter. In current state of code (main) BEEFY is partially integrated, so there exists pieces either in runtime or the node that are capable of driving the consensus, but they are not voting due there is no active beefy session key integrated yet |
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c85113521d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| pub grandpa: Grandpa, | ||
| // todo: add the beefy | ||
| // pub beefy: Beefy, | ||
| pub beefy: Beefy, |
There was a problem hiding this comment.
The parent and this runtime both advertise spec_version: 003_000_000, even though this changes the encoded SessionKeys shape and adds a storage migration. When this Wasm is applied to an existing 3.0.0 chain, util/toolkit/src/commands/runtime_upgrade.rs only recognizes spec > pre_spec_version, so the standard upgrade command eventually reports UpgradeNotEnacted; version-based metadata and runtime selection also cannot distinguish the old two-key runtime from this three-key runtime. Assign a new runtime/package/metadata version before shipping.
AGENTS.md reference: AGENTS.md:L69-L69
Useful? React with 👍 / 👎.
| AURA_SEED_FILE: "/seeds/aura.seed" | ||
| CROSS_CHAIN_SEED_FILE: "/seeds/cross_chain.seed" | ||
| GRANDPA_SEED_FILE: "/seeds/grandpa.seed" | ||
| BEEFY_SEED_FILE: "/seeds/beefy.seed" |
There was a problem hiding this comment.
This provisions the checked-in local environment, but the from-genesis and fork/mock paths generate validator overrides through genesisComposeOverride.ts and mockComposeOverride.ts, and both still create only AURA, GRANDPA, and cross-chain seed files/environment variables. Validators launched through those workflows therefore have no beef keystore entry for their advertised BEEFY key, so BEEFY finality cannot advance; both generators and their tests should set BEEFY_SEED_FILE, reusing the cross-chain seed where appropriate.
AGENTS.md reference: AGENTS.md:L69-L69
Useful? React with 👍 / 👎.
alba-press
left a comment
alba-press
left a comment
There was a problem hiding this comment.
Update from our side: we have now closed the full local verification chain against this PR's exact head c85113521de5a6e1f3aa1029bb49036285caddac.
On a fresh local chain with the corrected BEEFY authority history we reproduced:
user-owned shielded custom Zswap value
→ canonical user spend/nullifier
→ receiveShielded withdrawal receipt + user change
→ public withdrawal-intent binding
→ TxApplied
→ extrinsics_root + System::Events/state_root
→ MMR inclusion
→ BEEFY signed commitment
→ AuthoritiesProof
→ independent offline verifier = VALID
Concrete specimen: withdrawal at block 346, later BEEFY commitment at block 363 (validator_set_id=1 after a real session 0→1 transition), mmr_verifyProofStateless passed against the mh root, BEEFY signatures and AuthoritiesProof verified. No Ura signer/oracle/indexer/RPC verdict is used as validity authority.
So for us the remaining boundary is now specifically public Preprod activation/exposure, not local proof architecture. Our direct Preprod probes still see the BEEFY endpoint as not ready / no usable BEEFY justification path.
Could you help us pin the exact current activation blockers/order so we do not guess? In particular:
- Is merge/deployment of #2084 itself still the first required step for Preprod?
- Is #1953 (combined v1→v2 + BEEFY SessionKeys migration) still required in addition to #2084 for the existing Preprod state, or has #2084 superseded/absorbed that migration path?
- Is #2078 / #1895 (all-or-nothing session-key registration on committee rotation) considered a prerequisite for enabling BEEFY on public networks, or only related hardening?
- For Preprod activation, is the intended initial RPC surface
beefy_getFinalizedHead+ justifications +mmr_generateProof, or is the MMR-root-only / deduplicated committee format from MIP #262 expected to land first? - Is there any operator-side prerequisite (candidate
beefre-registration / keystore provisioning) that must happen before the runtime can safely activate BEEFY on Preprod?
We can provide the complete external conformance fixture and negative matrix from the local run if useful. The goal is to identify the real deployment blocker(s), not infer them from open PRs.
|
@alba-press I would call @MicroProofs to help with verifying my answers. |
|
Thanks @LGLO, this clears up most of the sequencing. @MicroProofs, could you help validate the remaining proof-side question for us? For a client verifying Midnight finality externally after #2084 is deployed, should we expect the usable path to be BEEFY justification, signed MMR root, Or does MIP #262 introduce a different committee/MMR proof format that clients should target instead? We are trying to determine whether the existing public BEEFY/MMR RPC path is the correct integration target once enabled, or whether we should wait for the MIP #262 light-client format before building against it. Thanks. This is the remaining dependency for our Preprod withdrawal verifier. |
4 participants
Overview
🗹 TODO before merging
📌 Submission Checklist
git commit -s) for the DCO🧪 Testing Evidence
Please describe any additional testing aside from CI:
🔱 Fork Strategy
Links