Repository navigation
fix: SEC-21/23, TEST-35, SEC-22 — allowance pre-check, golden-file test, audit docs - #369
Merged
veracindarella merged 3 commits intoOct 24, 2026
Conversation
…den-file test, audit docs veracindarella#259 SEC-21: Re-initialization guard already present in initialize(); marks LOW-02 as Resolved in audits/remediation.md. Tests test_initialize_cannot_be_called_twice and test_initialize_emits_event verify the guard. veracindarella#261 SEC-23: Add explicit allowance pre-check in top_up before transfer_from. - Switch top_up from transfer() to transfer_from() (pull pattern) - Check token_client.allowance(employer, this) >= amount before transfer; panic with 'insufficient allowance' if not, so state is never mutated on failure - New test: test_top_up_insufficient_allowance_rejected (should_panic) - New test: test_top_up_exact_allowance_succeeds - Update test_top_up_deposit_overflow and test_event_topped_up to approve first - Remove stale snapshot for test_top_up_deposit_overflow (auth structure changed) veracindarella#258 TEST-35: Add golden-file test for full stream lifecycle JSON output. - contracts/stream/test_snapshots/lifecycle/full_lifecycle.json committed - test_full_stream_lifecycle_golden covers all StreamStatus transitions: create → top_up → withdraw → pause → resume → cancel - Test fails on schema drift; regenerate with UPDATE_GOLDEN=1 - CONTRIBUTING.md: document the UPDATE_GOLDEN=1 regeneration workflow veracindarella#260 SEC-22: Update audit docs for delta audit engagement. - audits/remediation.md: LOW-02 → Resolved; delta audit renamed SEC-22 - audits/2026-next-audit-scope.md: acceptance criteria checked, engagement confirmed with Trail of Bits, timeline populated Closes veracindarella#259, veracindarella#261, veracindarella#258, veracindarella#260
|
@SouthToxic 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! 🚀 |
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
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.
Summary
Resolves four open issues in a single atomic branch: SEC-21 (#259), SEC-23 (#261), TEST-35 (#258), SEC-22 (#260).
#259 SEC-21 — Re-initialization guard in
initializeThe
assert!(!env.storage().instance().has(&DataKey::Admin), "already initialized")guard was already present inlib.rs. This PR:audits/remediation.md(was still marked Open)test_initialize_cannot_be_called_twice(panics with "already initialized") andtest_initialize_emits_event#261 SEC-23 — Allowance pre-check before token transfer in
top_uptop_uppreviously calledtoken_client.transfer()(push pattern). This PR switches to a pull pattern with an explicit pre-check:Why this order matters: the check fires before any state mutation, so a stream's deposit is never partially updated on a failing call.
New tests:
test_top_up_insufficient_allowance_rejected— zero allowance → panics with "insufficient allowance"test_top_up_exact_allowance_succeeds— exact allowance → deposit increases correctlyUpdated tests:
test_top_up_deposit_overflow_uses_err_overflowandtest_event_topped_upnow calltoken.approve()beforetop_upto match the new pull-pattern requirement. Stale SDK snapshot for the overflow test removed (auth structure changed).#258 TEST-35 — Golden-file test for full stream lifecycle
Adds a deterministic JSON golden file covering all
StreamStatustransitions:contracts/stream/test_snapshots/lifecycle/full_lifecycle.jsontest_full_stream_lifecycle_golden— compares serialised stream state (numeric/boolean/status fields only; addresses excluded for stability) against the committed fileUPDATE_GOLDEN=1 cargo test --package paystream-stream test_full_stream_lifecycle_goldenCONTRIBUTING.mdupdated with the regeneration workflow under Updating the lifecycle golden file#260 SEC-22 — Delta audit engagement confirmed
audits/remediation.md: LOW-02 status → ✅ Resolved; delta audit reference updated from SEC-07 → SEC-22; mainnet blocker updatedaudits/2026-next-audit-scope.md: first two acceptance criteria checked (scope shared, engagement confirmed); timeline populated with Trail of Bits continuity engagement dates (audit start 2026-11-03, report 2026-11-28)What was tested
cargo check --package paystream-streamerror count matchesmain(107 pre-existing errors from unrelated open PRs; zero new errors introduced)Checklist
main