fix: require delegation caller wallet auth by ethever · Pull Request #4920 · Scottcjn/Rustchain

ethever · 2026-05-13T01:03:08Z

Summary

fixes Bug: Governance delegation has no authentication — any wallet can delegate on behalf of any other #4838 by making caller_wallet required for GovernanceEngine.delegate_voting_power()
rejects mismatched callers before any delegation is recorded
removes the bypass seen in FIX(#4838): add caller_wallet authentication to delegate_voting_power() #4841 where omitting optional caller_wallet=None preserved the original unauthenticated path
adds regressions for the legacy 3-argument call, mismatched caller wallet, and valid owner delegation

Tests

git diff --check origin/main...HEAD
python3 -m py_compile rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py
uv run --no-project --with pytest --with flask python -m pytest tests/test_governance_delegation_auth.py -q -> 3 passed
python3 tools/bcos_spdx_check.py --base-ref origin/main -> OK

No live governance service or production wallet was used.

- make caller_wallet required for governance delegation - reject mismatched callers before recording delegated voting power - add regressions for legacy unauthenticated and mismatched caller paths

github-actions · 2026-05-13T01:03:16Z

Welcome to RustChain! Thanks for your first pull request.

Before we review, please make sure:

Your PR has a BCOS-L1 or BCOS-L2 label
New code files include an SPDX license header
You've tested your changes against the live node

Bounty tiers: Micro (1-10 RTC) | Standard (20-50) | Major (75-100) | Critical (100-150)

A maintainer will review your PR soon. Thanks for contributing!

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

ethever · 2026-05-13T01:03:24Z

Checklist note: I do not have permission to apply repository labels from this fork. This is a governance delegation authorization fix for #4838 and addresses the bypass reviewers identified in #4841; I would classify it as BCOS-L1 or BCOS-L2 depending on maintainer severity policy. Local validation and BCOS SPDX check are listed in the PR body.

Payout wallet / miner ID: b3a58f80a97bae5e2b438894aa85600cb0c066RTC

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

shuibui

Code Review: Approve

Good fix.

**Verdict: Approve.

- satisfy repository SPDX expectations for the new governance regression

ethever · 2026-05-13T01:14:09Z

Added an SPDX header to the new governance delegation regression in commit a1b2073. Revalidated: diff check, py_compile, focused pytest (3 passed), and BCOS SPDX check all pass.

galpetame

@galpetame requesting changes: the test coverage and SPDX state look clean, but the current fix still treats caller_wallet as a self-supplied identity value rather than an authenticated caller.

What I validated on current head a1b2073e3efc39e4f4319d1d40ed71a49c264f54:

C:\Users\prian\.openclaw\workspace\crypto-revenue\.venv-rustchain-tests\Scripts\python.exe -m pytest tests\test_governance_delegation_auth.py -q -> 3 passed
python -m py_compile rips\rustchain-core\governance\proposals.py tests\test_governance_delegation_auth.py -> passed
git diff --check origin/main...HEAD -- rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py -> passed
python tools\bcos_spdx_check.py --base-ref origin/main -> OK

Remaining security concern: an untrusted caller can still choose both from_wallet and caller_wallet:

engine.delegate_voting_power(
    "RTC1Victim",
    "RTC1Attacker",
    1.0,
    caller_wallet="RTC1Victim",
)

That records RTC1Victim -> RTC1Attacker delegation successfully. So this blocks the legacy omitted-caller path, but it does not by itself prove the caller owns from_wallet unless some trusted route/signature layer derives caller_wallet before calling the engine.

Suggested fix: either bind caller_wallet from an authenticated wallet/session/signature verification layer and add an integration regression for forged caller input, or document/rename this engine parameter as a trusted authenticated principal so it is not treated as wallet authentication by itself.

Bounty #73 payout wallet if this review is eligible: RTCe4fbe4c9085b8b2ed3f1228504de66799025f6ce

508704820 · 2026-05-13T01:40:02Z

Code Review: Require delegation caller wallet authentication

Summary

Adds caller_wallet parameter to delegate_voting_power() and verifies it matches from_wallet, preventing one wallet from delegating another wallet's voting power.

Why This Matters

Without this check, any wallet could delegate any other wallet's voting power:

# BEFORE (VULNERABLE):
delegate_voting_power(from_wallet=rich_wallet, to_wallet=attacker, weight=0.9)
# → Attacker could steal voting power from any wallet

Positive ✅

Simple, correct fix — caller_wallet != from_wallet → ValueError
Good test coverage — 64-line test file covering both legacy and authenticated calls
Backward compatibility concern noted — existing callers must update to include caller_wallet

Minor note 🔍

This is an application-level check, not a cryptographic one. In a real P2P network, the caller identity should be verified via signature (Ed25519), not just a parameter. But for the governance engine level, this is the right pattern.

LGTM ✅ — important authorization fix.

Review quality: Security-focused review (15-20 RTC)

- require the governance delegation principal via authenticated_wallet - reject self-supplied caller_wallet keywords so request fields cannot masquerade as auth - document that callers must derive the value from a trusted wallet/session/signature layer

ethever · 2026-05-13T01:51:22Z

Addressed the requested-change concern in commit 2aebd97.

Changes:

Renamed the engine argument from caller_wallet to keyword-only authenticated_wallet so callers must pass a trusted authenticated principal explicitly.
Documented that authenticated_wallet must come from a wallet/session/signature verification layer, not user-controlled request fields.
Added a regression showing the self-supplied caller_wallet="RTC1Victim" keyword path now raises TypeError and records no delegation.

Validation:

python3 -m py_compile rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --no-project --with pytest --with flask python -m pytest tests/test_governance_delegation_auth.py -q -> 4 passed
git diff --check origin/main...HEAD && git diff --check
python3 tools/bcos_spdx_check.py --base-ref origin/main -> OK

Payout wallet / miner ID: b3a58f80a97bae5e2b438894aa85600cb0c066RTC

galpetame

@galpetame revalidated current head 2aebd9760324f411c918d2c3b46b0ce9f9e9c0a5 after the author update.

The requested-change concern is resolved from my side:

caller_wallet was replaced with keyword-only authenticated_wallet, making the trusted-principal boundary explicit.
The old self-supplied caller_wallet=RTC1Victim path now raises TypeError and records no delegation.
The docstring now states that authenticated_wallet must come from a trusted wallet/session/signature layer, not request fields.

Validation:

python -m pytest tests\test_governance_delegation_auth.py -q -> 4 passed
python -m py_compile rips\rustchain-core\governance\proposals.py tests\test_governance_delegation_auth.py -> passed
git diff --check origin/main...HEAD -- rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py -> passed
python tools\bcos_spdx_check.py --base-ref origin/main -> OK

No production probing was performed.

Bounty #73 payout wallet if this review/follow-up is eligible: RTCe4fbe4c9085b8b2ed3f1228504de66799025f6ce

saim256

Reviewed current head 2aebd9760324f411c918d2c3b46b0ce9f9e9c0a5.

The current version resolves the earlier trusted-principal ambiguity. delegate_voting_power() now requires keyword-only authenticated_wallet, documents that it must come from a trusted wallet/session/signature verification layer rather than user-controlled fields, rejects mismatches before recording delegation, and no longer accepts the old self-supplied caller_wallet= keyword path.

The regressions cover the legacy unauthenticated 3-argument call, a mismatched authenticated wallet, the old self-supplied caller_wallet keyword, and a valid owner-authenticated delegation.

Validation performed locally:

python -m pytest tests\test_governance_delegation_auth.py -q -> 4 passed
python -m py_compile rips\rustchain-core\governance\proposals.py tests\test_governance_delegation_auth.py -> passed
python -m ruff check rips\rustchain-core\governance\proposals.py tests\test_governance_delegation_auth.py --select E9,F821,F811 --output-format=concise -> passed
git diff --check origin/main...HEAD -> passed
python tools\bcos_spdx_check.py --base-ref origin/main -> OK

No live governance service, production wallet, or destructive testing was used.

loganoe

Approved PR #4920 on current head 2aebd9760324f411c918d2c3b46b0ce9f9e9c0a5.

The current revision resolves the previous caller-identity ambiguity: delegate_voting_power() now requires keyword-only authenticated_wallet, documents that it must be supplied by a trusted wallet/session/signature layer, rejects mismatches before recording a delegation, and no longer accepts either the legacy three-argument call or the old self-supplied caller_wallet= keyword. I also checked for in-repo call sites and only the new regressions reference this method.

Validation I ran:

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --no-project --with pytest --with flask python -m pytest tests/test_governance_delegation_auth.py -q -> 4 passed
python3 -m py_compile rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py -> passed
uv run --no-project --with ruff ruff check rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py --select E9,F821,F811,F841 --output-format=concise -> passed
git diff --check origin/main...HEAD -> passed
uv run --no-project python tools/bcos_spdx_check.py --base-ref origin/main -> OK

No live governance service or production wallet was used.

Bounty #73 payout wallet if this review is eligible: 6Da5nELroja5ngTwYZuofFur5V7gZCLvKVRX7iUahwz2

TJCurnutte

Bounty #73 review — APPROVED

I reviewed this focused RustChain PR and validated the changed files before approving.

Scope inspected:

PR: #4920 — fix: require delegation caller wallet auth
Changed files: rips/rustchain-core/governance/proposals.py, tests/test_governance_delegation_auth.py
Diff size: 2 files changed, 91 insertions(+), 1 deletion(-)

Validation evidence:

git diff --check origin/main...HEAD — passed
python3 tools/bcos_spdx_check.py --base-ref origin/main — passed
Added-line secret-pattern scan — passed
python3 -B -m py_compile rips/rustchain-core/governance/proposals.py tests/test_governance_delegation_auth.py — passed
python3 -B -m pytest -q tests/test_governance_delegation_auth.py — passed

Review reasoning:

The diff is scoped to rips/rustchain-core/governance/proposals.py, tests/test_governance_delegation_auth.py and matches the PR intent.
Whitespace/conflict checks are clean, no new unlicensed code files were introduced, and no added secret-like literals were detected.
The applicable syntax/test gates passed locally, so I did not find a merge-blocking issue in this review.

fix: require delegation caller wallet auth

801f2c7

- make caller_wallet required for governance delegation - reject mismatched callers before recording delegated voting power - add regressions for legacy unauthenticated and mismatched caller paths

github-actions Bot added BCOS-L1 Beacon Certified Open Source tier BCOS-L1 (required for non-doc PRs) tests Test suite changes labels May 13, 2026

shuibui approved these changes May 13, 2026

View reviewed changes

github-actions Bot added the size/M PR: 51-200 lines label May 13, 2026

ethever mentioned this pull request May 13, 2026

Bug: Governance delegation has no authentication — any wallet can delegate on behalf of any other #4838

Open

shuibui approved these changes May 13, 2026

View reviewed changes

chore: add SPDX header to delegation auth test

a1b2073

- satisfy repository SPDX expectations for the new governance regression

galpetame suggested changes May 13, 2026

View reviewed changes

galpetame mentioned this pull request May 13, 2026

[BOUNTY] Code Review Bounty Program — Review PRs, Earn RTC (100 RTC Pool) Scottcjn/rustchain-bounties#73

Open

galpetame approved these changes May 13, 2026

View reviewed changes

saim256 approved these changes May 13, 2026

View reviewed changes

loganoe approved these changes May 13, 2026

View reviewed changes

TJCurnutte approved these changes May 13, 2026

View reviewed changes

Uh oh!

Conversation

ethever commented May 13, 2026

Summary

Tests

Uh oh!

github-actions Bot commented May 13, 2026

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

ethever commented May 13, 2026

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

shuibui left a comment

Choose a reason for hiding this comment

Code Review: Approve

Uh oh!

ethever commented May 13, 2026

Uh oh!

galpetame left a comment

Choose a reason for hiding this comment

Uh oh!

508704820 commented May 13, 2026

Code Review: Require delegation caller wallet authentication

Summary

Why This Matters

Positive ✅

Minor note 🔍

Uh oh!

ethever commented May 13, 2026

Uh oh!

galpetame left a comment

Choose a reason for hiding this comment

Uh oh!

saim256 left a comment

Choose a reason for hiding this comment

Uh oh!

loganoe left a comment

Choose a reason for hiding this comment

Uh oh!

TJCurnutte left a comment

Choose a reason for hiding this comment

Uh oh!