feat(reputation-contract): implement penalize_expert for dispute pena…#64
Merged
Bosun-Josh121 merged 1 commit intoLightForgeHub:mainfrom Mar 30, 2026
Merged
Conversation
📝 WalkthroughWalkthroughA new Changes
Sequence DiagramsequenceDiagram
participant Caller
participant Contract as Penalization Contract
participant Auth as Authorization
participant Storage as Storage Layer
participant Events as Event Emitter
Caller->>Contract: penalize_expert(expert, penalty_points)
Contract->>Auth: require_auth() on admin
Auth-->>Contract: ✓ Authorized
Contract->>Storage: get_expert_score(expert)
Storage-->>Contract: current_score
Contract->>Contract: new_score = max(0, current_score - penalty_points)
Contract->>Storage: set_expert_score(expert, new_score)
Storage-->>Contract: ✓ Persisted
Contract->>Events: expert_penalized(expert, penalty_points, new_score)
Events-->>Contract: ✓ Emitted
Contract-->>Caller: Result::Ok(())
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@contracts/reputation-scoring-contract/src/contract.rs`:
- Around line 58-63: The authorization currently only calls admin.require_auth()
after retrieving admin via storage::get_admin(env), missing the “admin or vault”
branch and never returning ReputationError::NotAuthorized; update the
authorization in the relevant function to: fetch both admin and vault (e.g.,
storage::get_admin(env) and storage::get_vault(env)), allow the call if either
admin.require_auth() succeeds or the vault authorization path succeeds (e.g.,
vault.require_auth() or an explicit check of env.message.sender == vault
address), and if neither authorizes the caller return
Err(ReputationError::NotAuthorized) instead of unconditionally calling
admin.require_auth(); ensure you reference and use the existing symbols admin,
vault, admin.require_auth(), and ReputationError::NotAuthorized when
implementing the branch.
In `@contracts/reputation-scoring-contract/src/test.rs`:
- Around line 79-99: The test test_penalize_expert_unauthorized currently
asserts is_err() too broadly; change it to capture the error from
client_strict.try_penalize_expert(&expert_strict, &50).unwrap_err() and assert
it specifically corresponds to ReputationError::NotAuthorized (error code 8).
Use the contract's error mapping (e.g., compare the returned
ContractError/Custom code or pattern-match to ReputationError::NotAuthorized) so
the test verifies the unauthorized path produces the exact
ReputationError::NotAuthorized rather than any failure.
- Around line 102-126: The two tests lack assertions and one assumes an initial
score of 100 without setting it; update
test_penalize_expert_score_underflow_protection to assert the expert’s score
stays at 0 after penalizing (use client.get_expert_score(&expert) and assert_eq!
to 0) and update test_penalize_expert_partial_deduction to explicitly initialize
the expert’s score to 100 (using client.set_expert_score(&expert, &100) or
client.reward_expert(&expert, &100) depending on available API), then assert
after the two penalize_expert calls that the final score equals 50 via
client.get_expert_score(&expert); keep references to setup(), client.init(),
penalize_expert(), get_expert_score(), and the initialization method you have in
the contract.
- Around line 56-76: The two tests (test_penalize_expert_by_admin and
test_penalize_expert_by_vault) currently call client.penalize_expert the same
way and only check for no panic, so they don't validate separate authorization
paths; update tests to explicitly simulate different callers and assert
authorization behavior: in test_penalize_expert_by_admin keep the default auth
context (no env.mock_all_auths) and call client.penalize_expert(&expert, &50)
and assert success, and in test_penalize_expert_by_vault use env.mock_all_auths
(or the appropriate mocking helper) to set the vault as the caller before
calling client.penalize_expert(&_expert, &50) and assert success; additionally
add a negative case for each test where the wrong caller is set and
client.penalize_expert returns or raises an unauthorized error (assert the
specific error/Result variant) so admin and vault authorization paths are
independently verified.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d92235cd-b057-454b-b345-b648d884ef66
📒 Files selected for processing (6)
contracts/reputation-scoring-contract/src/contract.rscontracts/reputation-scoring-contract/src/error.rscontracts/reputation-scoring-contract/src/events.rscontracts/reputation-scoring-contract/src/lib.rscontracts/reputation-scoring-contract/src/storage.rscontracts/reputation-scoring-contract/src/test.rs
Comment on lines
+58
to
+63
| // Verify contract is initialized | ||
| let admin = storage::get_admin(env).ok_or(ReputationError::NotInitialized)?; | ||
|
|
||
| // Require auth from admin (vault authorization is handled through admin) | ||
| admin.require_auth(); | ||
|
|
There was a problem hiding this comment.
Authorization logic misses the vault path and never returns NotAuthorized.
Line 62 enforces admin auth only. This does not satisfy the required “admin or vault” authorization behavior, and there is no branch returning ReputationError::NotAuthorized when caller is neither.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/reputation-scoring-contract/src/contract.rs` around lines 58 - 63,
The authorization currently only calls admin.require_auth() after retrieving
admin via storage::get_admin(env), missing the “admin or vault” branch and never
returning ReputationError::NotAuthorized; update the authorization in the
relevant function to: fetch both admin and vault (e.g., storage::get_admin(env)
and storage::get_vault(env)), allow the call if either admin.require_auth()
succeeds or the vault authorization path succeeds (e.g., vault.require_auth() or
an explicit check of env.message.sender == vault address), and if neither
authorizes the caller return Err(ReputationError::NotAuthorized) instead of
unconditionally calling admin.require_auth(); ensure you reference and use the
existing symbols admin, vault, admin.require_auth(), and
ReputationError::NotAuthorized when implementing the branch.
Comment on lines
+56
to
+76
| #[test] | ||
| fn test_penalize_expert_by_admin() { | ||
| let (env, admin, vault, client) = setup(); | ||
| client.init(&admin, &vault); | ||
| let expert = Address::generate(&env); | ||
|
|
||
| // Admin should be able to penalize | ||
| client.penalize_expert(&expert, &50); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_penalize_expert_by_vault() { | ||
| let (env, admin, vault, client) = setup(); | ||
| client.init(&admin, &vault); | ||
| let _expert = Address::generate(&env); | ||
|
|
||
| // Vault should be able to penalize | ||
| env.mock_all_auths(); | ||
| // Simulate vault calling by mocking auth context | ||
| client.penalize_expert(&_expert, &50); | ||
| } |
There was a problem hiding this comment.
Admin vs vault authorization is not actually distinguished in these tests.
Both tests execute the same call pattern and only verify “no error”, so they do not prove the vault-specific authorization path independently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/reputation-scoring-contract/src/test.rs` around lines 56 - 76, The
two tests (test_penalize_expert_by_admin and test_penalize_expert_by_vault)
currently call client.penalize_expert the same way and only check for no panic,
so they don't validate separate authorization paths; update tests to explicitly
simulate different callers and assert authorization behavior: in
test_penalize_expert_by_admin keep the default auth context (no
env.mock_all_auths) and call client.penalize_expert(&expert, &50) and assert
success, and in test_penalize_expert_by_vault use env.mock_all_auths (or the
appropriate mocking helper) to set the vault as the caller before calling
client.penalize_expert(&_expert, &50) and assert success; additionally add a
negative case for each test where the wrong caller is set and
client.penalize_expert returns or raises an unauthorized error (assert the
specific error/Result variant) so admin and vault authorization paths are
independently verified.
Comment on lines
+79
to
+99
| fn test_penalize_expert_unauthorized() { | ||
| let (env, admin, vault, client) = setup(); | ||
| client.init(&admin, &vault); | ||
| let _expert = Address::generate(&env); | ||
|
|
||
| // Create a new environment without mocking all auths to test unauthorized access | ||
| let env_strict = Env::default(); | ||
| let contract_id = env_strict.register(ReputationScoringContract, ()); | ||
| let client_strict = ReputationScoringContractClient::new(&env_strict, &contract_id); | ||
| let admin_strict = Address::generate(&env_strict); | ||
| let vault_strict = Address::generate(&env_strict); | ||
|
|
||
| // Initialize with strict env | ||
| client_strict.init(&admin_strict, &vault_strict); | ||
|
|
||
| let expert_strict = Address::generate(&env_strict); | ||
| let _unauthorized = Address::generate(&env_strict); | ||
|
|
||
| // Unauthorized address should not be able to penalize (no auth mocking for this env) | ||
| assert!(client_strict.try_penalize_expert(&expert_strict, &50).is_err()); | ||
| } |
There was a problem hiding this comment.
Unauthorized test should assert the specific contract error.
is_err() is too broad here; this should validate the unauthorized path maps to ReputationError::NotAuthorized (error code 8), not just any failure.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/reputation-scoring-contract/src/test.rs` around lines 79 - 99, The
test test_penalize_expert_unauthorized currently asserts is_err() too broadly;
change it to capture the error from
client_strict.try_penalize_expert(&expert_strict, &50).unwrap_err() and assert
it specifically corresponds to ReputationError::NotAuthorized (error code 8).
Use the contract's error mapping (e.g., compare the returned
ContractError/Custom code or pattern-match to ReputationError::NotAuthorized) so
the test verifies the unauthorized path produces the exact
ReputationError::NotAuthorized rather than any failure.
Comment on lines
+102
to
+126
| fn test_penalize_expert_score_underflow_protection() { | ||
| let (env, admin, vault, client) = setup(); | ||
| client.init(&admin, &vault); | ||
| let expert = Address::generate(&env); | ||
|
|
||
| // Penalize with more points than current score (default score is 0) | ||
| // Should result in score of 0, not underflow | ||
| client.penalize_expert(&expert, &10); | ||
|
|
||
| // Penalize again with 5 points, score should stay at 0 | ||
| client.penalize_expert(&expert, &5); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_penalize_expert_partial_deduction() { | ||
| let (env, admin, vault, client) = setup(); | ||
| client.init(&admin, &vault); | ||
| let expert = Address::generate(&env); | ||
|
|
||
| // First penalize to set a score (100 - 30 = 70) | ||
| client.penalize_expert(&expert, &30); | ||
|
|
||
| // Second penalize (70 - 20 = 50) | ||
| client.penalize_expert(&expert, &20); | ||
| } No newline at end of file |
There was a problem hiding this comment.
Score-behavior tests have no assertions and one invalid precondition.
These tests never assert resulting score values, and the “partial deduction” case assumes an initial score of 100 but never initializes it, so it does not verify the intended math.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/reputation-scoring-contract/src/test.rs` around lines 102 - 126,
The two tests lack assertions and one assumes an initial score of 100 without
setting it; update test_penalize_expert_score_underflow_protection to assert the
expert’s score stays at 0 after penalizing (use client.get_expert_score(&expert)
and assert_eq! to 0) and update test_penalize_expert_partial_deduction to
explicitly initialize the expert’s score to 100 (using
client.set_expert_score(&expert, &100) or client.reward_expert(&expert, &100)
depending on available API), then assert after the two penalize_expert calls
that the final score equals 50 via client.get_expert_score(&expert); keep
references to setup(), client.init(), penalize_expert(), get_expert_score(), and
the initialization method you have in the contract.
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
3 participants
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.
Adds penalize_expert() function to automatically dock reputation points when
vault admin resolves severe disputes. Protects against score underflow and
only allows authorized callers (admin). Includes event emission and tests.
Closes #55
Summary by CodeRabbit