feat: proof-bearing validator joins (restores #206, gated on its 4 companions) - #208
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CON-715 removed ValidatorWalletBlueprint.setOperator in favour of initiateOperatorTransfer + completeOperatorTransfer. The SDK only spoke the single-call surface, so rotating an operator against a consensus deployment that carries the change fails: the selector no longer exists, so viem reverts with no reason and the Python SDK reports ABIFunctionNotFound. Adds both calls plus cancelOperatorTransfer and the getPendingOperator view, and keeps setOperator for deployments that still expose it. The proof binding is the subtle part. validatorJoin's possession proof is verified by the ValidatorWalletFactory and is bound to it; rotation is verified by the wallet, so the registrar is the wallet's own address (PubKeyUtils.validateWithPossession(pubKey, address(this), owner(), proof)). createOperatorRegistration already takes the registrar as a parameter, so no change was needed there — but a join proof silently fails to verify for a rotation, which is the easy mistake to make. getOperatorTransferContext builds the wallet-bound context and reads owner() from the wallet rather than assuming the caller is the owner, so a mismatch surfaces as a clear local error instead of an onlyOwner revert. A test pins that a join-bound proof does not verify for rotation while a wallet-bound one does. Callers still need updating; this only adds the surface they need: genlayer-e2e's driver interface, genlayer-cli's staking set-operator, and genlayer-py's staking_set_operator all remain single-call. Verified: npx tsc --noEmit, npm test -- --run (123/123, no type errors), npm run build.
CON-715 widened both staking structs — Claim gained `offset`, Commit gained `outstanding`/`priced`/`fragmented` and narrowed several members — while keeping the same function names and arguments. Static tuples decode positionally, so reading a post-CON-715 chain with the shape in STAKING_ABI does not fail: `commit.input` picks up `claim.commit`. Pending deposits came back as small indices instead of amounts, which is why the e2e delegator scenarios asserted 0.1 GEN and got "0" or "2". Both layouts are deployed — consensus v0.6-dev has the old one, #1290 the new — so neither shape can simply replace the other. STAKING_COMMIT_VIEWS_CURRENT_ABI adds the post-CON-715 shape for the four struct-returning views, and readCommitView resolves which one the chain speaks, once, then caches it for the client: getStakeInfo loops over every pending entry and must not re-probe per entry. The probe only works in one direction, and the tests pin that: decoding a legacy response with the current shape throws (the response is short), while decoding a current response with the legacy shape succeeds and lies. So the current shape is always attempted first and a decode *failure* identifies a legacy chain. If that asymmetry ever breaks, the probe would quietly start reporting wrong balances again — hence the test asserting the exact misread value rather than merely "not equal". Also drops duplicate ValidatorWalletBlueprint entries: the ABI already carried initiateOperatorTransfer/completeOperatorTransfer/cancelOperatorTransfer/ getPendingOperator, with initiateOperatorTransfer declared as taking an address rather than the pubkey and possession proof it actually takes. Verified: npx tsc --noEmit, npm test -- --run (126/126, no type errors), npm run build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores the change from #206, which was reverted in #207 because it merged alone, ahead of the four PRs that make it work.
The code is unchanged — this branch reverts the revert, so the tree is byte-identical to the original
6f127388. Nothing needs re-reviewing on its merits.Draft on purpose
validatorJoinrequires{registration}plus a client-sideverifyOperatorRegistration. Every caller must send a proof, and the chain must be able to verify it. Until all five land together, an SDK that demands the proof breaks every consumer — which is exactly what happened: both e2e lanes failed on the sharedjoin as a validatorstep withOperator registration proof does not match the owner, registrar, chain, or public key, and the deployed chain could not have satisfied it either.tracks/v0.6/matrix.yamlpinsgenlayer-js: v2-devas a moving branch, so anything merged here reaches every downstream build immediately. That is what turned a one-repo merge into a cross-repo outage.Depends-On — all four must land with this
The consensus side is the hard constraint: no caller can succeed until the chain can verify the proof. Repointed from #1285 to #1290, which is what the rest of the set now pins — genlayer-node#1707, cli#405, e2e#716, explorer#485 and dev-env#123 all resolve consensus to #1290. Leaving #1285 here also broke dispatch mechanically: the closure allows one ref per repository per track, so #1285 from this body collided with #1290 from the consumers.
Before un-drafting
gate_policy: release-branchesmakes e2e advisory on*-dev, so fix: support proof-bearing validator joins #206'sE2E Testscheck sat permanently pending and it merged on the unit-test job alone. That gate is why nothing objected the first time; treat a pending e2e here as a blocker, not a pass.