feat: M010 CosmWasm reputation-signal contract with unit tests#71
feat: M010 CosmWasm reputation-signal contract with unit tests#71brawlaphant wants to merge 1 commit intoregen-network:mainfrom
Conversation
Implements the M010 Reputation Signal mechanism as a CosmWasm smart contract for Regen Network. Includes signal submission with activation delay, challenge/dispute lifecycle (submit, resolve, escalate), admin invalidation, v0 decay-weighted score computation, bond enforcement, arbiter management, and 38 passing unit tests covering all spec acceptance criteria. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the reputation-signal CosmWasm contract, which manages a lifecycle for stake-weighted endorsement signals, including submission, activation, and a challenge/dispute mechanism. Several critical issues were identified during the review: the use of floating-point arithmetic (f64) and transcendental functions in reputation score calculations poses a risk to blockchain consensus due to potential non-determinism; the storage of signal IDs in a Vec for subjects is not scalable and could lead to gas limit exhaustion; and the string-based composite keys for subjects are vulnerable to collisions. Additionally, the implementation lacks the required minimum stake verification for signal submission, and the active challenges query is inefficient as it filters the entire challenge set in memory.
|
|
||
| /// Index: (subject_type_str, subject_id, category) -> Vec<signal_id> | ||
| /// We store a composite key as a string: "{subject_type}:{subject_id}:{category}" | ||
| pub const SUBJECT_SIGNALS: Map<&str, Vec<u64>> = Map::new("subject_signals"); |
There was a problem hiding this comment.
Storing a Vec<u64> as the value in SUBJECT_SIGNALS is not scalable. As the number of signals for a subject grows, loading and saving this entire list will eventually exceed gas limits. Consider changing this to a composite key Map, such as Map<(&str, u64), ()>, where the key is (subject_key, signal_id). This allows you to iterate over signals for a subject using .prefix() without loading the entire list into memory.
| let half_life = config.decay_half_life_seconds as f64; | ||
| let lambda = (2.0_f64).ln() / half_life; | ||
|
|
||
| let mut w_sum: f64 = 0.0; | ||
| let mut d_sum: f64 = 0.0; | ||
| let mut contributing: u32 = 0; | ||
|
|
||
| for id in &ids { | ||
| if let Ok(signal) = SIGNALS.load(deps.storage, *id) { | ||
| if !signal.status.contributes_to_score() { | ||
| continue; | ||
| } | ||
| contributing += 1; | ||
|
|
||
| let age_secs = now.seconds().saturating_sub(signal.submitted_at.seconds()) as f64; | ||
| let decay = (-lambda * age_secs).exp(); | ||
| let w = signal.endorsement_level as f64 / 5.0; | ||
|
|
||
| w_sum += w * decay; | ||
| d_sum += decay; | ||
| } | ||
| } |
There was a problem hiding this comment.
The use of f64 and transcendental functions like ln() and exp() for reputation score computation is risky in a blockchain environment. Floating-point operations can lead to non-deterministic results across different hardware architectures or compiler versions, potentially causing consensus issues. It is highly recommended to use fixed-point arithmetic (e.g., cosmwasm_std::Decimal) for all scoring logic. For the exponential decay, you can implement a fixed-point approximation or use a lookup table.
| fn exec_submit_signal( | ||
| deps: DepsMut, | ||
| env: Env, | ||
| info: MessageInfo, | ||
| subject_type: SubjectType, | ||
| subject_id: String, | ||
| category: String, | ||
| endorsement_level: u8, | ||
| evidence: Evidence, | ||
| ) -> Result<Response, ContractError> { |
There was a problem hiding this comment.
The exec_submit_signal function does not verify if the signaler meets the minimum stake requirement for the category, which is a requirement specified in the SPEC.md (Section 5.4 and Acceptance Test 2). While the PR description mentions that stake weighting is omitted in v0, the minimum stake gating is a separate security mechanism to prevent signal spam. You should query the staking module or bank balance via deps.querier to enforce this requirement.
| fn subject_key(subject_type: &SubjectType, subject_id: &str, category: &str) -> String { | ||
| format!("{}:{}:{}", subject_type, subject_id, category) | ||
| } |
There was a problem hiding this comment.
The subject_key helper generates a string key by concatenating subject_type, subject_id, and category with colons. This approach is vulnerable to key collisions if any of the input strings contain colons themselves. A safer approach is to use a composite key tuple (SubjectType, &str, &str) directly with cw-storage-plus Maps, which handles serialization and prefixing safely without risk of collision.
| for result in CHALLENGES | ||
| .range(deps.storage, Some(Bound::inclusive(start)), None, Order::Ascending) | ||
| { | ||
| let (_, challenge) = result?; | ||
| if matches!(challenge.outcome, ChallengeOutcome::Pending) { | ||
| challenges.push(challenge); | ||
| if challenges.len() >= limit { | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This query iterates through the CHALLENGES map and filters for Pending status in memory. If there are many resolved challenges, this will become inefficient and may exceed gas limits. Consider maintaining a separate index for active challenges or using a status-prefixed key in the storage map to allow direct iteration over pending challenges.
1 participant
Summary
contracts/reputation-signal/)target/andCargo.lockto.gitignorefor Rust build artifactsContract structure
src/state.rssrc/msg.rssrc/error.rssrc/contract.rssrc/tests.rsSpec coverage
Test plan
cargo testpasses all 38 testscargo buildcompiles cleanly with no warningsGenerated with Claude Code