fix(sortition): derive committee seed after request [skip-line-limit] - #1792
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
hmzakhalid
force-pushed
the
fix/future-sortition-entropy
branch
from
August 12, 2026 12:44
5ff7988 to
ae8541d
Compare
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe registry now commits a future entropy block for each committee request. The committee seed is resolved from that block hash and the E3 ID after confirmation. Rust sortition, contracts, SDK types, tests, deployment settings, and documentation now use separate computation and committee seeds. ChangesDelayed entropy sortition
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Requester
participant CiphernodeRegistryOwnable
participant CiphernodeRegistrySolReader
participant Sortition
Requester->>CiphernodeRegistryOwnable: requestCommittee(legacySeed)
CiphernodeRegistryOwnable->>CiphernodeRegistryOwnable: record next block as entropyBlock
CiphernodeRegistrySolReader->>CiphernodeRegistrySolReader: wait for confirmations and derive committee seed
CiphernodeRegistrySolReader->>Sortition: forward CommitteeRequested with resolved seed
Sortition->>Sortition: rank tickets using committee seed
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
hmzakhalid
marked this pull request as ready for review
August 12, 2026 12:52
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/interfold-contracts/contracts/interfaces/ICiphernodeRegistry.sol (1)
73-84: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRegenerate the checked-in registry artifacts.
CiphernodeRegistryOwnable.json, itsartifacts.d.ts, and both mock artifacts still defineCommitteeRequestedwithseedand withoutticketPrice. Regenerate these artifacts to match the six-argument event emitted byCiphernodeRegistryOwnable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/interfold-contracts/contracts/interfaces/ICiphernodeRegistry.sol` around lines 73 - 84, Regenerate the checked-in registry artifacts for CommitteeRequested so CiphernodeRegistryOwnable.json, its artifacts.d.ts, and both mock artifacts match the six-argument event definition: use entropyBlock instead of seed and include ticketPrice. Do not alter the Solidity event declaration.packages/interfold-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json (1)
754-754: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPreserve
LicenseTransferShortfallin a legacy ABI.The package publishes and exports its artifacts. Consumers using the current ABI cannot decode historical logs for this event.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/interfold-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json` at line 754, Preserve the LicenseTransferShortfall event definition in the published legacy ABI artifact represented by IBDondingRegistry.json, restoring its complete ABI entry so consumers can decode historical logs. Keep the existing artifact structure and event signature consistent with the contract interface.
🧹 Nitpick comments (1)
crates/evm/src/ciphernode_registry/actor.rs (1)
216-218: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
Ok(None)branch is unreachable.
parse_registry_logreturnsOk(Some(..))on every success path, and aNonefrom the extractors becomes an error through.context(..)?at Line 152.InterfoldEvmEvent::Processed(id)is therefore never produced. Change the return type toResult<EvmEvent>and drop the branch, or document the case that is meant to yieldOk(None).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/evm/src/ciphernode_registry/actor.rs` around lines 216 - 218, Update the parsing flow around parse_registry_log and the parsed match so its return type is Result<EvmEvent>, remove the unreachable Ok(None) and InterfoldEvmEvent::Processed(id) branch, and preserve the existing error propagation for extractor failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.md`:
- Around line 107-112: Update the requestCommittee call flow so the legacy
argument is explicitly defined: reuse the existing seed symbol as the legacySeed
argument, or pass seed directly and identify the callee parameter as
legacy-only. Ensure the trace no longer shows an undefined legacySeed and
clearly preserves the ABI-compatible input.
In `@crates/ciphernode-builder/src/ciphernode_builder.rs`:
- Line 1040: Update the reorg_confirmations default in the chain configuration
flow to a non-zero value so reorg protection remains enabled when the setting is
unset. Preserve explicitly configured values, including zero if supported, and
ensure the derived confirmation value is passed through the existing
seed/entropy handling path.
In `@crates/evm/src/ciphernode_registry/actor.rs`:
- Around line 103-146: Bound the entropy-block retry loop in the reader flow
around the `let seed = loop` block with the proposed `ENTROPY_WAIT_TIMEOUT`
deadline, declared alongside `EVENT_FORWARD_TIMEOUT`. On expiry, stop retrying
and reject the current log with a named failure so `ctx.wait` does not suspend
mailbox processing indefinitely; preserve the existing provider reconnect and
successful seed derivation behavior.
In `@crates/sortition/src/sortition/actor.rs`:
- Around line 50-53: Persist and restore sortition_seeds and pending_requests
across actor restarts, or deterministically rebuild them from durable E3 and
registry events before enabling effects; update the actor initialization and
event-handling paths around these fields while preserving E3Requested replay
gating. Add restart coverage for both request-before-seed and
seed-before-request orders, asserting exactly one ticket-generation attempt per
E3.
In
`@packages/interfold-contracts/contracts/registry/CiphernodeRegistryOwnable.sol`:
- Around line 626-675: Prevent sortition seed loss when no ticket is submitted
before the blockhash retention horizon by adding a permissionless
resolveSortitionSeed(uint256 e3Id) entrypoint that invokes _resolveSortitionSeed
and persists the result. Ensure callers can pin the seed once sortitionSeed
reports it ready, while preserving the existing resolved-seed behavior used by
submitTicket.
In `@packages/interfold-sdk/src/events/types.ts`:
- Line 88: Update the SDK release metadata for the public CommitteeRequestedData
field rename from seed to entropyBlock: add release notes describing the
breaking change and bump the SDK version before publishing, following the
event-schema compatibility guidance in agent/INVARIANTS.md.
---
Outside diff comments:
In
`@packages/interfold-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.json`:
- Line 754: Preserve the LicenseTransferShortfall event definition in the
published legacy ABI artifact represented by IBDondingRegistry.json, restoring
its complete ABI entry so consumers can decode historical logs. Keep the
existing artifact structure and event signature consistent with the contract
interface.
In `@packages/interfold-contracts/contracts/interfaces/ICiphernodeRegistry.sol`:
- Around line 73-84: Regenerate the checked-in registry artifacts for
CommitteeRequested so CiphernodeRegistryOwnable.json, its artifacts.d.ts, and
both mock artifacts match the six-argument event definition: use entropyBlock
instead of seed and include ticketPrice. Do not alter the Solidity event
declaration.
---
Nitpick comments:
In `@crates/evm/src/ciphernode_registry/actor.rs`:
- Around line 216-218: Update the parsing flow around parse_registry_log and the
parsed match so its return type is Result<EvmEvent>, remove the unreachable
Ok(None) and InterfoldEvmEvent::Processed(id) branch, and preserve the existing
error propagation for extractor failures.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 159a169e-ac7d-4274-a803-b154561ae8b8
📒 Files selected for processing (38)
README.mdagent/INVARIANTS.mdagent/flow-trace/00_INDEX.mdagent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.mdcrates/ciphernode-builder/src/ciphernode_builder.rscrates/events/src/interfold_event/e3_requested.rscrates/evm/src/ciphernode_registry/actor.rscrates/evm/src/ciphernode_registry/events.rscrates/evm/src/contracts.rscrates/sortition/Readme.mdcrates/sortition/src/sortition/actor.rscrates/sortition/src/sortition/handlers/lifecycle.rscrates/sortition/src/sortition/handlers/registry.rscrates/sortition/src/sortition/handlers/request.rsdocs/pages/ciphernode-operators/tickets-and-sortition.mdxdocs/pages/computation-flow.mdxdocs/pages/internals/sortition.mdxdocs/pages/tutorials/operator-troubleshooting.mdxpackages/interfold-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.jsonpackages/interfold-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.jsonpackages/interfold-contracts/artifacts/contracts/interfaces/IInterfold.sol/IInterfold.jsonpackages/interfold-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.jsonpackages/interfold-contracts/artifacts/contracts/token/InterfoldTicketToken.sol/InterfoldTicketToken.jsonpackages/interfold-contracts/contracts/Interfold.solpackages/interfold-contracts/contracts/interfaces/ICiphernodeRegistry.solpackages/interfold-contracts/contracts/registry/CiphernodeRegistryOwnable.solpackages/interfold-contracts/deploy/protocol/example.protocol.config.jsonpackages/interfold-contracts/scripts/deployInterfold.tspackages/interfold-contracts/test/E3Lifecycle/E3Integration.spec.tspackages/interfold-contracts/test/E3Lifecycle/Sortition.spec.tspackages/interfold-contracts/test/Pricing/DustRotation.spec.tspackages/interfold-contracts/test/Pricing/PullPaymentsAndAllowlist.spec.tspackages/interfold-contracts/test/Registry/CiphernodeRegistryOwnable.spec.tspackages/interfold-contracts/test/Slashing/CommitteeExpulsion.spec.tspackages/interfold-contracts/test/fixtures/constants.tspackages/interfold-contracts/test/fixtures/helpers.tspackages/interfold-sdk/src/events/types.tspackages/interfold-sdk/tests/events.test.ts
hmzakhalid
force-pushed
the
fix/future-sortition-entropy
branch
from
August 12, 2026 13:08
ae8541d to
910ded0
Compare
hmzakhalid
force-pushed
the
fix/future-sortition-entropy
branch
from
August 12, 2026 13:18
910ded0 to
4b48a76
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
crates/evm/src/ciphernode_registry/actor.rs (1)
353-360: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
attachinherits the zero-confirmation default.
attachforwards tosetup, so every caller ofattachgetsconfirmations = 0and no provider factory. Add the confirmation depth and the optional factory to this signature, or document thatattachis for tests only.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/evm/src/ciphernode_registry/actor.rs` around lines 353 - 360, Update CiphernodeRegistrySolReader::attach to accept and forward the confirmation depth and optional provider factory to setup, ensuring callers do not inherit the zero-confirmation default. Preserve the existing processor and provider parameters while keeping the setup path consistent with the expanded configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/evm/src/ciphernode_registry/actor.rs`:
- Around line 98-154: Change the committee-request entropy resolution around the
tokio::time::timeout block so expiration schedules the log for retry instead of
propagating the timeout through with_context into InterfoldEvmEvent::Rejected.
Move the potentially 300-second entropy wait, including derive_sortition_seed
resolution, outside the actor’s synchronous mailbox handling so CiphernodeAdded,
TicketSubmitted, and CommitteePublished events continue processing while it
retries.
- Around line 192-195: Update CiphernodeRegistrySolReader::setup to avoid the
zero-confirmation default by requiring or supplying a non-zero confirmation
depth, and update attach to accept and forward confirmations plus the optional
ProviderFactory; alternatively restrict attach to test usage as requested. Apply
the changes at crates/evm/src/ciphernode_registry/actor.rs lines 192-195 and
353-360, ensuring all attach callers provide the intended chain-read
configuration.
---
Nitpick comments:
In `@crates/evm/src/ciphernode_registry/actor.rs`:
- Around line 353-360: Update CiphernodeRegistrySolReader::attach to accept and
forward the confirmation depth and optional provider factory to setup, ensuring
callers do not inherit the zero-confirmation default. Preserve the existing
processor and provider parameters while keeping the setup path consistent with
the expanded configuration.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 86210264-c7a3-4cb9-ba77-e7faa151e711
📒 Files selected for processing (5)
agent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.mdcrates/ciphernode-builder/src/ciphernode_builder.rscrates/evm/src/ciphernode_registry/actor.rscrates/sortition/src/sortition/actor.rsdocs/pages/internals/sortition.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/sortition/src/sortition/actor.rs
- crates/ciphernode-builder/src/ciphernode_builder.rs
- docs/pages/internals/sortition.mdx
- agent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.md
hmzakhalid
force-pushed
the
fix/future-sortition-entropy
branch
from
August 12, 2026 14:00
4b48a76 to
69f8522
Compare
hmzakhalid
enabled auto-merge (squash)
August 12, 2026 14:10
hmzakhalid
disabled auto-merge
August 12, 2026 14:24
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
1 participant
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.
Summary
Fixes zenith-security/2026-07-interfold#5
Notes
This closes requester revert grinding. It uses chain-native block-hash entropy, so it does not claim the stronger block-producer resistance of VRF.
Testing
Summary by CodeRabbit
New Features
Bug Fixes
Documentation