Conversation
WalkthroughIntroduces a new counting module for the governance package that implements vote tallying functionality with quorum management, voter tracking, and support for Against/For/Abstain vote types. Includes trait definitions, storage layer, comprehensive test coverage, and constants for TTL semantics. Changes
Sequence DiagramsequenceDiagram
participant Voter
participant CountingTrait as Counting Trait
participant Storage
participant Event as Event System
Voter->>CountingTrait: count_vote(proposal_id, account, vote_type, weight)
CountingTrait->>Storage: has_voted(proposal_id, account)
Storage-->>CountingTrait: bool (false = can vote)
CountingTrait->>Storage: get_proposal_vote_counts(proposal_id)
Storage-->>CountingTrait: ProposalVoteCounts
CountingTrait->>Storage: Update vote tally by weight
Storage->>Storage: Increment for/against/abstain
CountingTrait->>Storage: Mark HasVoted(proposal_id, account)
Storage-->>CountingTrait: vote recorded
CountingTrait-->>Voter: success
Note over Voter,Storage: Later: Check voting outcome
Voter->>CountingTrait: tally_succeeded(proposal_id)
CountingTrait->>Storage: get_proposal_vote_counts(proposal_id)
Storage-->>CountingTrait: ProposalVoteCounts
CountingTrait->>CountingTrait: Compare for_votes > against_votes
CountingTrait-->>Voter: bool (success status)
Voter->>CountingTrait: quorum_reached(proposal_id)
CountingTrait->>Storage: get_proposal_vote_counts(proposal_id)
Storage-->>CountingTrait: ProposalVoteCounts
CountingTrait->>Storage: get_quorum()
Storage-->>CountingTrait: u128
CountingTrait->>CountingTrait: Check (for + abstain) >= quorum
CountingTrait-->>Voter: bool (quorum status)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #568 +/- ##
==========================================
- Coverage 96.24% 96.23% -0.01%
==========================================
Files 57 58 +1
Lines 5507 5581 +74
==========================================
+ Hits 5300 5371 +71
- Misses 207 210 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/governance/src/counting/storage.rs`:
- Around line 263-268: The vote-count and has-voted entries are written with
e.storage().persistent().set(...) but do not extend TTL; after calling
e.storage().persistent().set(&CountingStorageKey::ProposalVote(proposal_id.clone()),
&counts) and after e.storage().persistent().set(&voted_key, &true) call the
storage's extend_ttl() for the same keys (i.e., invoke
e.storage().persistent().extend_ttl(&vote_key) and
e.storage().persistent().extend_ttl(&voted_key) or the equivalent API) so both
newly-created entries get their TTL extended to match the codebase TTL policy.
| // Store updated vote counts | ||
| let vote_key = CountingStorageKey::ProposalVote(proposal_id.clone()); | ||
| e.storage().persistent().set(&vote_key, &counts); | ||
|
|
||
| // Mark account as having voted | ||
| e.storage().persistent().set(&voted_key, &true); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check the actual file and lines in question
fd -t f "storage.rs" packages/governance/src/counting/ | head -1 | xargs wc -lRepository: OpenZeppelin/stellar-contracts
Length of output: 121
🏁 Script executed:
# Read the relevant section
cat -n packages/governance/src/counting/storage.rs | sed -n '260,270p'Repository: OpenZeppelin/stellar-contracts
Length of output: 470
🏁 Script executed:
# Find TTL constant definitions
rg "COUNTING_EXTEND_AMOUNT|COUNTING_TTL_THRESHOLD" packages/governance/src/counting/Repository: OpenZeppelin/stellar-contracts
Length of output: 898
🏁 Script executed:
# Search for extend_ttl usage patterns in the codebase
rg "extend_ttl" packages/governance/src/counting/ -A 2 -B 2Repository: OpenZeppelin/stellar-contracts
Length of output: 1411
🌐 Web query:
soroban-sdk persistent storage set method TTL behavior
💡 Result:
In soroban-sdk, calling env.storage().persistent().set(&key, &val) writes the value but does not give you control over (or explicitly “refresh”) the entry’s TTL—TTL management is a separate concern you must handle yourself. The network assigns/maintains TTL, and you must periodically extend it to keep entries live. [1]
To manage TTL for a persistent entry, use:
env.storage().persistent().extend_ttl(&key, threshold, extend_to)which extends only if the current TTL is belowthreshold, and then sets it toextend_to(in ledgers). [2]- TTL policies are monotonic: they extend TTL, and cannot reduce/reset an entry to a shorter lifetime. [3]
Also note: when a persistent entry’s TTL reaches 0, it becomes archived (not deleted forever) and must be restored before it can be accessed again. [1]
Sources
[1] Stellar Docs — State archival (TTL rules; persistent entries archive at TTL=0): https://developers.stellar.org/docs/learn/fundamentals/contract-development/storage/state-archival
[2] soroban-sdk Rust docs — Persistent::extend_ttl semantics: https://docs.rs/soroban-sdk/latest/soroban_sdk/storage/struct.Persistent.html
[3] OpenZeppelin audit note — TTL can only be extended, not shortened/reset: https://www.openzeppelin.com/news/stellar-contracts-library-v0.3.0-rc.2-audit
Extend TTL when writing vote and HasVoted entries.
The persistent().set() calls on lines 265 and 268 do not extend TTL. Without an explicit extend_ttl() call, newly created entries rely only on the network's minimum TTL and could expire before the proposal ends, causing vote tallies to become inaccessible and enabling double-voting. This is inconsistent with the codebase's TTL management pattern (which extends TTL on reads). Call extend_ttl() immediately after both set() operations:
🛠️ Suggested fix
// Store updated vote counts
let vote_key = CountingStorageKey::ProposalVote(proposal_id.clone());
e.storage().persistent().set(&vote_key, &counts);
+ e.storage().persistent().extend_ttl(
+ &vote_key,
+ COUNTING_TTL_THRESHOLD,
+ COUNTING_EXTEND_AMOUNT,
+ );
// Mark account as having voted
e.storage().persistent().set(&voted_key, &true);
+ e.storage().persistent().extend_ttl(
+ &voted_key,
+ COUNTING_TTL_THRESHOLD,
+ COUNTING_EXTEND_AMOUNT,
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Store updated vote counts | |
| let vote_key = CountingStorageKey::ProposalVote(proposal_id.clone()); | |
| e.storage().persistent().set(&vote_key, &counts); | |
| // Mark account as having voted | |
| e.storage().persistent().set(&voted_key, &true); | |
| // Store updated vote counts | |
| let vote_key = CountingStorageKey::ProposalVote(proposal_id.clone()); | |
| e.storage().persistent().set(&vote_key, &counts); | |
| e.storage().persistent().extend_ttl( | |
| &vote_key, | |
| COUNTING_TTL_THRESHOLD, | |
| COUNTING_EXTEND_AMOUNT, | |
| ); | |
| // Mark account as having voted | |
| e.storage().persistent().set(&voted_key, &true); | |
| e.storage().persistent().extend_ttl( | |
| &voted_key, | |
| COUNTING_TTL_THRESHOLD, | |
| COUNTING_EXTEND_AMOUNT, | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/governance/src/counting/storage.rs` around lines 263 - 268, The
vote-count and has-voted entries are written with
e.storage().persistent().set(...) but do not extend TTL; after calling
e.storage().persistent().set(&CountingStorageKey::ProposalVote(proposal_id.clone()),
&counts) and after e.storage().persistent().set(&voted_key, &true) call the
storage's extend_ttl() for the same keys (i.e., invoke
e.storage().persistent().extend_ttl(&vote_key) and
e.storage().persistent().extend_ttl(&voted_key) or the equivalent API) so both
newly-created entries get their TTL extended to match the codebase TTL policy.
1 participant
Fixes #564
PR Checklist
Summary by CodeRabbit