feat(resilient_civic_participation): poc#73
Conversation
2ba940d to
e467df8
Compare
e9e41d2 to
7c5e0fc
Compare
oskarth
left a comment
There was a problem hiding this comment.
Left three follow-up comments on the lifecycle/dispute fixes from 019353f.
| /// when bountyToken is denominated near 1:1 with ETH (e.g., WETH). | ||
| /// For tokens with different decimals the wei-vs-token-unit | ||
| /// comparison treats wei numerically; the cap still bounds payout. | ||
| function markUnresolved(bytes32 petitionId) external { |
There was a problem hiding this comment.
Could we split the resolve and markUnresolved deadlines? Right now both become callable at closeAtBlock + RESOLUTION_DEADLINE_BLOCKS, so a caller can tombstone/refund the petition in the same block range where a valid resolver is first allowed to submit. A short grace period after resolution opens, or separate resolution_open_at / unresolved_at thresholds, would avoid that race.
| PetitionRecord storage rec = petitions[dp.petitionId]; | ||
| _advanceStateOnRead(rec); | ||
| // SPEC line 107: dispute only during DisputeWindow. | ||
| if (rec.state != PetitionState.DisputeWindow) revert InvalidState(); |
There was a problem hiding this comment.
Should disputes also close once resolution opens? The new resolve gate prevents early resolution, but _preflightDispute still accepts disputes for the whole DisputeWindow, including after closeAtBlock + RESOLUTION_DEADLINE_BLOCKS. If the intent is that resolution happens after the dispute period is over, this path probably needs a matching upper-bound check.
| if (pi.slot != rec.slot) revert PriorStateMismatch(); | ||
| } | ||
|
|
||
| function _commitBatch(BatchPublicInputs calldata pi) internal { |
There was a problem hiding this comment.
Could we revisit how re-publication after rollback is indexed? _cascadeRepudiation resets nextBatchIndex to the disputed index, but _commitBatch still appends the replacement batch to the array while emitting the reset logical index. Later dispute(batchIndex) indexes directly into batches[petitionId], so the emitted index and storage index can diverge after a rollback. Either truncating/superseding the tail or separating logical batch ids from array positions would make this easier to reason about.
37aba02 to
e7105cd
Compare
oskarth
left a comment
There was a problem hiding this comment.
Not good to go yet. The Solidity lifecycle fixes are present, but the Rust shadow registry still implements the old boundaries.
pocs/civic-participation/resilient-civic-participation/src/registry/core.rs:601:disputeonly checksPetitionState::DisputeWindow; it never rejects atclose_at_block + RESOLUTION_DEADLINE_BLOCKS.SPEC.md:110and SolidityPetitionRegistry.sol:800-810now close disputes at the resolution-open block.pocs/civic-participation/resilient-civic-participation/src/registry/core.rs:812:mark_unresolvedgates onclose_at_block + RESOLUTION_DEADLINE_BLOCKS, but the spec and Solidity now require+ MARK_UNRESOLVED_GRACE_BLOCKSbefore tombstoning/refund. This preserves the old resolver/markUnresolved race in the in-memory state machine.
Please mirror the new lifecycle constants and add Rust regression tests for both boundaries.
Verified locally:
forge soldeer installforge testpasses: 38/38cargo test --libpasses: 132 passed, 2 ignored
Full cargo test is still blocked locally by missing circuits/signer/target/vk_hash; the harness says to run scripts/generate-verifiers.sh first, and this machine does not have nargo installed.
Converting this to a non-blocking suggestion; original review tone/state was too strong.
|
Suggestion for a follow-up: the Solidity lifecycle changes look reflected in In particular:
Would suggest either mirroring those constants/checks in the Rust registry, or adding a short note if the shadow registry is intentionally looser than the contract. Regression tests around those two boundary blocks would make this easier to keep aligned. |
oskarth
left a comment
There was a problem hiding this comment.
LGTM, thanks for mirroring the lifecycle boundaries in the Rust registry and adding regression coverage. The prior follow-up is addressed on my side.
Meyanis95
left a comment
There was a problem hiding this comment.
I haven't read all the code; I mostly checked the circuits, and it looks good. Only comment is on the dispute types (esp. 0x02 and 0x03) that can't be reached as circuit already enforces these conditions, but this does not enable any soundness or completeness bug.
USE_MOCK_VERIFIERS=false cargo test --test golden_path --release -- --nocaptureA petition system where signers prove eligibility against an external ResilientIdentity credential root, sign once per petition with cross-petition unlinkability, and produce an outcome verifiable from durable chain state after the dispute window closes. Key properties:
N = 2^24slot values. After each finalised signing the slot's seed is overwritten in place; an audit-time device compromise reveals nothing about past signed slots beyond what the prior seed material had already produced.(petition_id, slot, class_tag, nullifier, identity_tag). The same RI leaf signs different petitions under distinct slot indices and distinct per-slot ratchet values; no on-chain linkability between two signings of the same signer beyond the predicate-match intersection bound.close_at_block + dispute window, anyone can reconstruct the leaf setLfrom blob bytes, recompute(b, b_per_class), and re-verify the resolution SNARK. The organiser, RI issuer, relayers, and resolver can all go offline; the chain holds enough state to settle the outcome.