Description
contracts/governance/src/storage.rs defines get_nonce/increment_nonce and a DataKey::Nonce(Address) storage key, documented as "Nonce for replay-protection on delegation signatures." A full-text search confirms increment_nonce is never called anywhere in the crate, and get_nonce has no caller either — no function in delegation.rs or lib.rs reads or advances a nonce before performing any state-changing operation. Soroban's require_auth() (used throughout delegation.rs/lib.rs) already provides replay protection via the account's on-chain sequence number, so this on-chain Nonce mechanism is currently inert. Without clarifying doc comments, a future contributor could reasonably assume it is live security infrastructure and build on top of an assumption that never actually applies at runtime.
Requirements and Context
- Must not remove or rename
DataKey::Nonce, get_nonce, or increment_nonce — this issue is documentation-only; a separate decision to remove or wire them up is out of scope here.
- Must not change any storage layout (
DataKey enum ordering matters for #[contracttype] serialization).
Suggested Execution
Branch: docs/clarify-vestigial-nonce-storage
Implement Changes
- Update the doc comment on
DataKey::Nonce(Address) in contracts/governance/src/types.rs to state plainly that this key is currently unused by any contract entry point, and that replay protection for require_auth-gated calls is already provided by Soroban's account sequence numbers — this field is reserved and not part of the active security model.
- Add matching doc comments above
get_nonce/increment_nonce in contracts/governance/src/storage.rs explaining the same, and pointing to delegation.rs's security-properties doc comment (which already lists require_auth() as the auth mechanism) so the two don't appear to contradict each other.
- Add a short note to the module-level doc comment at the top of
storage.rs's "Nonces (replay protection)" section clarifying current status.
Test and Commit
Run cargo test --lib (and cargo test for proptest/fuzz targets where relevant) in the appropriate contracts/<governance|token-factory> directory, confirming the new coverage passes and cargo build --target wasm32v1-none --release --lib still succeeds.
Example Commit Message
docs(contract): clarify the vestigial on-chain Nonce mechanism in storage.rs
Closes #<issue>
Guidelines
- Branch from
main, open a PR back to main
- All new code must have corresponding tests
- Run
cargo fmt --check, cargo clippy, and cargo test --lib before pushing
- Follow existing naming conventions and file structure
- PR description must reference this issue number (e.g.,
Closes #<issue>)
- Keep commits atomic and use conventional commit format
Description
contracts/governance/src/storage.rsdefinesget_nonce/increment_nonceand aDataKey::Nonce(Address)storage key, documented as "Nonce for replay-protection on delegation signatures." A full-text search confirmsincrement_nonceis never called anywhere in the crate, andget_noncehas no caller either — no function indelegation.rsorlib.rsreads or advances a nonce before performing any state-changing operation. Soroban'srequire_auth()(used throughoutdelegation.rs/lib.rs) already provides replay protection via the account's on-chain sequence number, so this on-chainNoncemechanism is currently inert. Without clarifying doc comments, a future contributor could reasonably assume it is live security infrastructure and build on top of an assumption that never actually applies at runtime.Requirements and Context
DataKey::Nonce,get_nonce, orincrement_nonce— this issue is documentation-only; a separate decision to remove or wire them up is out of scope here.DataKeyenum ordering matters for#[contracttype]serialization).Suggested Execution
Branch:
docs/clarify-vestigial-nonce-storageImplement Changes
DataKey::Nonce(Address)incontracts/governance/src/types.rsto state plainly that this key is currently unused by any contract entry point, and that replay protection forrequire_auth-gated calls is already provided by Soroban's account sequence numbers — this field is reserved and not part of the active security model.get_nonce/increment_nonceincontracts/governance/src/storage.rsexplaining the same, and pointing todelegation.rs's security-properties doc comment (which already listsrequire_auth()as the auth mechanism) so the two don't appear to contradict each other.storage.rs's "Nonces (replay protection)" section clarifying current status.Test and Commit
Run
cargo test --lib(andcargo testfor proptest/fuzz targets where relevant) in the appropriatecontracts/<governance|token-factory>directory, confirming the new coverage passes andcargo build --target wasm32v1-none --release --libstill succeeds.Example Commit Message
Guidelines
main, open a PR back tomaincargo fmt --check,cargo clippy, andcargo test --libbefore pushingCloses #<issue>)