Skip to content

Fix/single frame auth - #1465

Merged
greatest0fallt1me merged 13 commits into
Predictify-org:masterfrom
akinboyewaSamson:fix/single-frame-auth
Oct 24, 2026
Merged

greatest0fallt1me merged 13 commits into
Predictify-org:masterfrom
akinboyewaSamson:fix/single-frame-auth

Conversation

@akinboyewaSamson

Copy link
Copy Markdown
Contributor

closes #1457

Search Tools

List pull requests
Completed with input: {
"owner": "akinboyewaSamson",
"repo": "predictify-contracts",
"head": "akinboyewaSamson:fix/single-frame-auth",
"state": "all",
"perPage": 10,
"fields": [
"number",
"title",
"body",
"state",
"html_url",
"head",
"base",
"created_at"
]
}

Get commit details
Completed with input: {
"owner": "akinboyewaSamson",
"repo": "predictify-contracts",
"sha": "d6220d1c1c79e8459df770e1cd9ba4541e0c1d8f",
"detail": "stats"
}

Ran terminal command: cargo test -p predictify-hybrid --lib
Tool execution failed with exit code 101

Ran terminal command: cargo check -p predictify-hybrid --no-default-features
Tool execution failed with exit code 101

Below is a copy-ready PR description using the repository template. Replace #<issue number> before submitting; no issue number was provided. It also notes that this branch only contains some of the entrypoints described in the original request, and that tests and builds have not passed locally.

# Pull Request Description

## 📋 Basic Information

### Type of Change
Please select the type of change this PR introduces:

- [x] 🐛 Bug fix (non-breaking change which fixes an issue)
- [ ] ✨ New feature (non-breaking change which adds functionality)
- [ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [x] 📚 Documentation update
- [x] 🧪 Test addition/update
- [ ] 🔧 Refactoring (no functional changes)
- [ ] ⚡ Performance improvement
- [x] 🔒 Security fix
- [ ] 🎨 UI/UX improvement
- [ ] 🚀 Deployment/Infrastructure change

### Related Issues
<!-- Link to related issues using keywords like "Closes", "Fixes", "Resolves" -->
Closes #<issue number>
Fixes N/A
Related to N/A

### Priority Level
- [ ] 🔴 Critical (blocking other development)
- [ ] 🟡 High (significant impact)
- [x] 🟢 Medium (moderate impact)
- [ ] 🔵 Low (minor improvement)

---

## 📝 Detailed Description

### What does this PR do?
Removes duplicate authentication for the applicable dispute and fee call chains present in this checkout. Entrypoints authenticate once, then delegate to crate-private `*_authorized` manager methods. The manager methods retain their business and applicable permission checks but do not call `require_auth()` again.

| Entrypoint | Before | After |
| --- | --- | --- |
| `dispute_market` | Entrypoint and dispute manager both called `user.require_auth()` | Entrypoint authenticates once; manager uses `process_dispute_authorized` |
| `vote_on_dispute` | Entrypoint and dispute manager both called `user.require_auth()` | Entrypoint authenticates once; manager uses `vote_on_dispute_authorized` |
| `resolve_dispute` | Entrypoint and dispute manager both called `admin.require_auth()` | Entrypoint authenticates and checks primary-admin identity; manager uses `resolve_dispute_authorized` and retains its permission check |
| `collect_fees` | Entrypoint authenticated; manager authenticated again in non-test builds | Entrypoint authenticates and checks primary-admin identity; manager uses `collect_fees_authorized` and retains its permission check |

Adds generated-client regression tests using `mock_auths` with a single auth entry for the signer, covering successful dispute/vote state changes and authorization/business error paths. Adds `docs/TESTING.md` and indexes it from `docs/README.md`.

**Scope limitation:** This checkout does not contain the treasury update, force-resolve, oracle-weight, history-cap, or anti-grief entrypoints listed in the original request. No active `#[ignore]` test attributes were present to restore. Consequently, this PR only addresses the applicable paths currently present; it does not implement or verify those absent paths.

### Why is this change needed?
Soroban can reject repeated `require_auth()` calls for the same address in one invocation frame with `Error(Auth, ExistingValue)`. Authenticating at the entrypoint and delegating to an authorized internal method avoids the duplicate authorization while preserving the managers' business and permission checks.

### How was this tested?
Formatting and whitespace checks passed:

- `rustfmt --edition 2021 --check contracts\predictify-hybrid\src\single_frame_auth_tests.rs` — passed
- `git diff --check` — passed

The Rust test and check commands were attempted but could not compile because this Windows environment has no `link.exe`; tests did not execute:

```text
$ cargo test -p predictify-hybrid --lib
error: linker `link.exe` not found
...
exit code 101

$ cargo check -p predictify-hybrid --no-default-features
error: linker `link.exe` not found
...
exit code 101

$ cargo test -p predictify-hybrid --lib -- --ignored
error: linker `link.exe` not found
...
exit code 101

A GNU-target check was also blocked by Windows Application Control (os error 4551). CI, coverage, full/slim WASM builds, and WASM size have not been verified.

Alternative Solutions Considered

Keeping authentication in both the entrypoint and manager triggers the duplicate-auth hazard. Removing authentication from the entrypoint would leave it outside the contract boundary. This change instead keeps auth at the entrypoint and makes the authorized manager methods crate-private.


🏗️ Smart Contract Specific

Contract Changes

Please check all that apply:

  • Core contract logic modified
  • Oracle integration changes (Pyth/Reflector)
  • New functions added
  • Existing functions modified
  • Storage structure changes
  • Events added/modified
  • Error handling improved
  • Gas optimization
  • Access control changes
  • Admin functions modified
  • Fee structure changes

Oracle Integration

  • Pyth oracle integration affected
  • Reflector oracle integration affected
  • Oracle configuration changes
  • Price feed handling modified
  • Oracle fallback mechanisms
  • Price validation logic

Market Resolution Logic

  • Hybrid resolution algorithm changed
  • Dispute mechanism modified
  • Fee structure updated
  • Voting mechanism changes
  • Community weight calculation
  • Oracle weight calculation

Security Considerations

  • Access control reviewed
  • Reentrancy protection
  • Input validation
  • Overflow/underflow protection
  • Oracle manipulation protection

🧪 Testing

Test Coverage

  • Unit tests added/updated
  • Integration tests added/updated
  • All tests passing locally
  • Manual testing completed
  • Oracle integration tested
  • Edge cases covered
  • Error conditions tested
  • Gas usage optimized
  • Cross-contract interactions tested

Test Results

# Focused contract tests were attempted; compilation was blocked before tests ran.
cargo test -p predictify-hybrid --lib
# Result: failed before test execution; linker `link.exe` not found (exit code 101)

# Required no-default-features check was attempted.
cargo check -p predictify-hybrid --no-default-features
# Result: failed before checking the package; linker `link.exe` not found (exit code 101)

# Ignored-test listing command was attempted.
cargo test -p predictify-hybrid --lib -- --ignored
# Result: failed before test execution; linker `link.exe` not found (exit code 101)

# Focused formatting check
rustfmt --edition 2021 --check contracts\predictify-hybrid\src\single_frame_auth_tests.rs
# Result: passed

# Whitespace/error check
git diff --check
# Result: passed

Manual Testing Steps

  1. Added generated-client tests using mock_auths with one signer authorization entry per invocation.
  2. Added assertions for successful dispute/vote state changes and token transfer paths.
  3. Added assertions for mismatched signer, non-admin identity, and business-error behavior. Tests could not be executed locally because the linker is unavailable.

📚 Documentation

Documentation Updates

  • README updated
  • Code comments added/updated
  • API documentation updated
  • Examples updated
  • Deployment instructions updated
  • Contributing guidelines updated
  • Architecture documentation updated

Breaking Changes

Breaking Changes:

  • None intended. Contract entrypoint signatures and storage layout are unchanged.
  • Internal manager methods are now crate-private authorized variants.

Migration Guide:

No migration required.


🔍 Code Quality

Code Review Checklist

  • Code follows Rust/Soroban best practices
  • Self-review completed
  • No unnecessary code duplication
  • Error handling is appropriate
  • Logging/monitoring added where needed
  • Security considerations addressed
  • Performance implications considered
  • Code is readable and well-commented
  • Variable names are descriptive
  • Functions are focused and small

Performance Impact

  • Gas Usage: Expected unchanged or slightly reduced by removing repeated auth work; not measured.
  • Storage Impact: None; no storage layout changes.
  • Computational Complexity: No material change expected.

Security Review

  • No obvious security vulnerabilities
  • Access controls properly implemented
  • Input validation in place
  • Oracle data properly validated
  • No sensitive data exposed

🚀 Deployment & Integration

Deployment Notes

  • Network: Testnet/Mainnet
  • Contract Address: Not applicable
  • Migration Required: No
  • Special Instructions: Run the focused tests and no-default-features check in CI/a Rust environment with a working linker before merge.

Integration Points

  • Frontend integration considered
  • API changes documented
  • Backward compatibility maintained
  • Third-party integrations updated

📊 Impact Assessment

User Impact

  • End Users: Dispute operations in the affected paths should no longer fail from duplicate auth.
  • Developers: Internal authorized manager methods cannot be called from outside the crate.
  • Admins: Dispute resolution and fee collection retain entrypoint authentication and admin identity checks.

Business Impact

  • Revenue: No direct fee or revenue model change.
  • User Experience: Unblocks the affected dispute flows where repeated authorization caused invocation failure.
  • Technical Debt: Adds focused regression tests and documents the single-auth pattern.

✅ Final Checklist

Pre-Submission

  • Code follows Rust/Soroban best practices
  • All CI checks passing
  • No breaking changes (or breaking changes are documented)
  • Ready for review
  • PR description is complete and accurate
  • All required sections filled out
  • Test results included
  • Documentation updated

Review Readiness

  • Self-review completed
  • Code is clean and well-formatted
  • Commit messages are clear and descriptive
  • Branch is up to date with main
  • No merge conflicts

📸 Screenshots (if applicable)

Not applicable.

🔗 Additional Resources

  • Design Document: N/A
  • Technical Spec: docs/TESTING.md
  • Related Discussion: N/A
  • External Documentation: Soroban SDK authorization behavior

💬 Notes for Reviewers

Please pay special attention to:

  • The placement of authentication at the contract entrypoint and the crate-private authorized manager calls.
  • Regression tests use a single signer auth entry and avoid mock_all_auths.

Questions for reviewers:

  • Please confirm CI test/check results once run in an environment with the required linker.
  • The additional treasury/oracle/history/anti-grief paths described in the original request are absent from this branch; please confirm whether they belong in a separate change.

Thank you for your contribution to Predictify! 🚀

@drips-wave

drips-wave Bot commented Oct 22, 2026

Copy link
Copy Markdown

@akinboyewaSamson Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

akinboyewaSamson and others added 2 commits October 24, 2026 08:37
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
akinboyewaSamson and others added 8 commits October 24, 2026 08:53
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@greatest0fallt1me
greatest0fallt1me merged commit 0d7bc4e into Predictify-org:master Oct 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants