Skip to content

Remove the double require_auth in admin and dispute entrypoints that trap on-chain with Auth, ExistingValue #1457

Closed

Description

@greatest0fallt1me

Summary

docs/TESTING.md records that calling require_auth() twice for the same address in one frame makes the host reject the invocation with Error(Auth, ExistingValue), and that this is "a real runtime hazard". The initialize and collect_fees instances were fixed. At least ten other entrypoints still authenticate in lib.rs and then call a manager that authenticates the same address again, so they cannot succeed on-chain. This issue removes the second authorization in each and adds a guard so the pattern cannot come back.

Background and evidence

I traced these by following each lib.rs entrypoint into its callee:

Entrypoint in lib.rs First auth Second auth
set_treasury (~2065) admin.require_auth() TreasuryTimelockManager::queue_update (recovery.rs ~670), and cancel_update (~736) on the retry branch, which makes three
queue_treasury_update / apply_treasury_update / cancel_treasury_update (~2097–2117) entrypoint auth recovery.rs queue_update / apply_update (~704) / cancel_update
force_resolve_market (~2830) require_primary_admin force_resolve::mark_resolved (force_resolve.rs ~90)
set_oracle_weight (~3425) admin.require_auth() OracleIntegrationManager::set_oracle_weight (oracles.rs ~3270)
vote_on_dispute (~4068) user.require_auth() DisputeManager::vote_on_dispute (disputes.rs ~1623)
resolve_dispute (~4110) require_primary_admin DisputeManager::resolve_dispute (disputes.rs ~1156)
set_history_cap (~4134) require_primary_admin DisputeManager::set_history_cap (disputes.rs ~773)
set_anti_grief_floor (~4146) require_primary_admin DisputeManager::set_anti_grief_floor (disputes.rs ~791)

DisputeManager::process_dispute (~934) already shows the intended pattern: the comment "calls user.require_auth() before delegating here".

Roughly 26 of the tests ignored with #[ignore = "known failing ..."] fail with Auth, ExistingValue (per docs/TESTING.md). Some of them exercise these paths.

Scope

In scope

  1. Fix each pair. Keep authentication at exactly one layer, preferably the entrypoint, with require_primary_admin or require_admin_permission. Change the inner manager functions to *_authorized variants that assume auth was done. Keep their identity and permission checks, without require_auth.
  2. No public inner auth paths. Mark the inner variants pub(crate), so that no future entrypoint can skip auth by calling them directly.
  3. Un-ignore the tests. Remove #[ignore] from the ignored tests that fail only because of these double-auth paths, and fix any fixture drift.
  4. Guard against recurrence. Add a test, or a small build.rs-free unit test that walks a list of entrypoints, that calls each one through the generated client using real (non-mocked) auth via env.mock_auths(&[MockAuth { ... }]) with a single auth entry. That is how on-chain behaviour is reproduced.
  5. Docs. Update docs/TESTING.md: the ignored count and the "remaining double-auth" note.

Out of scope

  • upgrade_contract / rollback_upgrade (tracked in the slim-build upgradeability issue).
  • Adding new permissions or roles.

Design constraints

  • One require_auth per address per frame on every listed path.
  • Error precedence is unchanged: auth, then identity (Unauthorized), then validation, then state.
  • Use real auth in tests. mock_all_auths() hides this class of bug. The new tests must use mock_auths with exactly one entry per invocation.
  • Size. The full and slim builds both compile. Report the slim WASM size; it is expected to be unchanged or smaller.

Test plan

Add contracts/predictify-hybrid/src/single_frame_auth_tests.rs.

# Scenario Expected
1 Each of the 10 entrypoints called with a single MockAuth for the correct address Ok; expected state change
2 Same call with an auth entry for a different address auth error; no state change
3 set_treasury when a pending update exists (the retry branch) succeeds with one auth entry
4 vote_on_dispute by a non-participant with valid auth the existing business error, not an auth error
5 resolve_dispute by a non-admin with valid auth Unauthorized
6 Previously ignored double-auth tests pass without #[ignore]
7 Inner *_authorized functions not reachable from the contract client (compile-time)

Regression proof. Row 1 for vote_on_dispute and set_treasury fails on current master with Error(Auth, ExistingValue). Paste the failure in the PR.

Acceptance criteria

  • No listed entrypoint reaches require_auth twice for the same address (the PR includes a call-chain table).
  • The ignored-test count in docs/TESTING.md drops by the number of tests you restored, and cargo test -p predictify-hybrid --lib -- --ignored shows those tests are gone from the ignored set.
  • cargo test -p predictify-hybrid --lib passes.
  • cargo check -p predictify-hybrid --no-default-features passes.
  • Coverage is at least 95% on the touched inner functions.

Getting started

git checkout -b fix/single-frame-auth
cargo test -p predictify-hybrid --lib -- --ignored 2>&1 | grep -B2 ExistingValue | head -50

Read these first:

  • docs/TESTING.md (the double-auth explanation)
  • lib.rs: require_primary_admin, require_admin_permission
  • disputes.rs: process_dispute, which shows the correct pattern

PR requirements

  • Link this issue (Closes #<n>).
  • Include the call-chain table before and after.
  • Paste the test output, including the restored tests, and the coverage.
  • Example commit: fix(auth): authenticate once per frame in dispute and treasury entrypoints.

Priority rationale

Medium. Each fix is contained, but together they unblock ten admin and dispute entrypoints that cannot run on-chain, and restore part of the ignored test suite. It requires understanding Soroban's auth recording, beyond what mock_all_auths shows.

Activity

  1. aliyuHabibu commented on Oct 18, 2026

    @aliyuHabibu
    Contributor

    @aliyuHabibu has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    Hi maintainer, i reviewed the issue and this is exactly the type of work i enjoy, i can deliver a clean, well-tested PR promptly, please assign it to me.

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @aliyuHabibu to this issue.

  2. akinboyewaSamson commented on Oct 19, 2026

    @akinboyewaSamson
    Contributor

    @akinboyewaSamson has applied to work on this issue as part of the Stellar Wave Program's 10th wave.

    hello maintainer, i would love to work on this

    ℹ️ Repo Maintainers: To accept this application, review their application or assign @akinboyewaSamson to this issue.

  3. drips-wave commented on Oct 19, 2026

    @drips-wave

    Congratulations, @akinboyewaSamson! 🎉 Your application was accepted by the repo's maintainers, and the issue is due on October 22, 2026.

    🧑‍💻 @akinboyewaSamson: Please resolve the issue such that the repo's maintainers have enough time to review your contribution before the due date. You'll earn Points for completing the issue on-time, which will make you eligible for a share of the Stellar Wave Program's reward pool.

    Warning

    When opening a PR, please link it to this issue to ensure it gets tracked accurately. Points are awarded when this issue is marked as completed by the maintainer.

    🤠 Repo maintainers: Please keep an eye on the contributor's progress and review their work before the due date. You can manage this issue, including adjusting its complexity and points, here.

    🌊 Happy Wave 🌊

  4. grantfox-oss commented on Oct 24, 2026

    @grantfox-oss

    🎉 This issue has been marked as completed on GrantFox!

    @akinboyewaSamson's PR #1465 was approved and merged by @greatest0fallt1me.

    🏆 @akinboyewaSamson: You earned 40 FoxPoints for this contribution! Your current tier: Builder (2,185 total points). Track your full progress on GrantFox.

    👏 Great work, @akinboyewaSamson! Keep contributing to Predictify-org.

  5. drips-wave commented on Oct 24, 2026

    @drips-wave

    This issue has been completed by @akinboyewaSamson as part of the Stellar Wave Program's 10th Wave 🥳

    😎 @akinboyewaSamson: You earned 100 Points for completing this issue! Once the Wave's rewards are finalized, you'll be eligible for a percentage of the Wave's reward pool based on the percentage of total points you've earned. Learn more here. You can also Leave a review to share your experience working on this issue.

    🧑‍💻 Repo maintainers: How'd the contributor do? Leave a review to share your experience working with them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Stellar WaveStellar Wave Program issuepriority:mediumMedium implementation difficultysecuritySecurity hardeningsmart-contractSoroban smart-contract worksorobanSoroban SDK / Stellar

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions