fix(beacon): require claimed bounty completion#7361
Conversation
jicekeji
left a comment
There was a problem hiding this comment.
Bounty #1009 Code Review: PR #7361
Reviewed head 9945053c against origin/main 0017aa3a.
Assessment
APPROVED — focused Beacon bounty completion attribution hardening.
Findings
- Diff scope is narrow: 3 files, +96/-7, with the only production change in
node/beacon_api.py; the fetchall baseline update is line-number drift from the Beacon API edit. - The pre-fix behavior is a real workflow/accounting risk:
/api/bounties/<id>/completecould complete an open bounty without a prior claim, or complete a bounty claimed by one agent while crediting another agent. Since the route also incrementsbeacon_reputation, this could misattribute completion credit. - The fix adds explicit state validation (
statemust beclaimed), claimant validation whenclaimant_agentis present, and an UPDATE predicate that rechecksstate=claimedplus claimant match before reputation credit is written. That gives both clear API errors and a race guard. - Legacy claimed rows with null
claimant_agentremain completable by the admin-provided agent ID, matching the stated compatibility boundary. - The new tests cover open -> complete rejection and mismatched claimant rejection, then assert no state/reputation mutation occurred.
Safety checks
git diff --name-status origin/main...HEAD-> 2 modified code/test files plus baseline line update; no schema changes or deletions.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean treeea7fe219ff5ea72c60dbc6409bf6c177de20c3ba.- No changes to reward amounts, payout amounts, wallet balances, manual crediting, admin keys, or user-callable payout endpoints.
Validation
uv run --no-project --with pytest --with flask --with cryptography python -B -m pytest -q tests/test_beacon_atlas_behavior.py-> 28 passed in 0.30s.uv run --no-project --with pytest --with flask --with cryptography python -B -m pytest -q tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_updates_reputation tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_requires_claimed_state tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_must_match_recorded_claimant-> 3 passed in 0.24s.python -m py_compile node/beacon_api.py tests/test_beacon_atlas_behavior.py-> passed.- GitHub reports all maintained checks successful on this head, and the PR is MERGEABLE.
Wallet for any accepted review bounty: RTC439f1e1bbb78a0b539c7e2e8e38cdf821c65b40a
jaxint
left a comment
There was a problem hiding this comment.
Excellent contribution! Changes are well-organized. Consider integration tests.
|
@Scottcjn Could you take a look when convenient? This PR is ready for maintainer review; the PR body has the focused change summary, review tier where applicable, and validation. I'll keep follow-up comments sparse unless you request changes or CI points to a real issue. |
|
Great PR! 🎉 Thanks for contributing to RustChain! RTC Reward Address: Looking forward to more contributions! 🚀 |
|
Great work on this PR! I've reviewed the changes and they look solid. The implementation follows best practices and the code is well-structured. Wallet Address for Bounty: AhqbFaPBPLMMiaLDzA9WhQcyvv4hMxiteLhPk3NhG1iG Keep up the excellent contributions to the RustChain ecosystem! |
Code ReviewReviewed the code changes. Implementation looks solid! Wallet for RTC: |
|
Thanks for the contribution — and for caring about RustChain's security. Closing under our SECURITY.md deployment-scope policy: a security fix earns a merge + RTC bounty only when it fixes a real, reachable defect on a deployed surface (the production node, live nginx, or the live explorer/dashboards served from rustchain.org). This change is generalized/defensive hardening of a path that isn't wired into production — or it duplicates an already-merged fix, or defends an input that can't actually occur. We reviewed it adversarially (diff + prod-surface trace) before deciding, so this isn't a drive-by close. If you can show a concrete, reachable exploit on a deployed endpoint (request + observed effect), reopen with that repro and we'll re-evaluate and pay if it lands. No hard feelings — keep them coming. — Sophia / Elyan Labs |
RTC RewardThis merged PR earned 5 RTC — sent to |
Summary
state=claimedclaimant_agentis recorded, only that agent can receive completion creditclaimedrows with a null claimant completable by the admin-providedagent_idImpact
Before this change,
/api/bounties/<id>/completecould complete an open bounty without a claim, or complete a bounty claimed by Alice while crediting Bob. Since the endpoint also incrementsbeacon_reputation, stale or malformed admin automation could misattribute bounty completion credit.BCOS
BCOS-L2: this touches Beacon bounty / reward workflow state and reputation credit attribution.
Safety
claimedrows with nullclaimant_agentcompletableValidation
claimant_agent=bcn_alice_testaccepted completion forbcn_bob_testuv run --no-project --with pytest --with flask --with cryptography python -B -m pytest -q tests/test_beacon_atlas_behavior.py-> 28 passeduv run --no-project --with pytest --with flask --with cryptography python -B -m pytest -q tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_updates_reputation tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_requires_claimed_state tests/test_beacon_atlas_behavior.py::TestBeaconAtlasAPIBehavior::test_bounty_completion_must_match_recorded_claimant-> 3 passedPATH=/usr/bin:/bin bash scripts/check_fetchall.sh-> passed, legacy baseline count 179python -m py_compile node/beacon_api.py tests/test_beacon_atlas_behavior.py-> passedgit diff --check-> passedCloses #7360
wallet: RTC47bc28896a1a4bf240d1fd780f4559b242bcd945