Repository navigation
Fix: 4 issues (#107, #109, #112, #113) - #150
Merged
Busiii-adetiba merged 2 commits intoOct 18, 2026
Merged
Conversation
Co-authored-by: Simple Agent <agent@simple.dev>
|
@k2ghostyou 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.
Closes #107
Closes #109
Closes #112
Closes #113
Issues
#107 [cross-cutting] Add cargo audit (or equivalent dependency audit) to CI
#109 [cross-cutting] Add #[contracttype] size checks to CI so Lease and Subscription encoding changes are caught
#112 [cross-cutting] Error code numbering differs between contracts (AlreadyInitialised is 8 in registry, 7 in lease, 9 in subsc...
#113 [docs and tooling] SECURITY_AUDIT.md lists SV-7 (admin rotation) as resolved, but the access-control matrix has no row for set...
Summary
The CI workflow change should keep the audit job independent of the build so a new advisory does not block unrelated PRs unexpectedly, but it must still fail the run. For the contracttype size tests, use a deterministic representative value (fixed addresses/strings via testutils) so the golden size is stable; document in the test that the golden value must be bumped deliberately when the type changes. Error-discriminant tests should live in each contract's existing #[cfg(test)] module. The docs table for #112 should explicitly state that codes are never renumbered and that cross-contract consumers must not assume a shared numbering.
Changes
cargo-auditjob that installs Rust stable, installscargo-audit(e.g.cargo install cargo-audit --locked), and runscargo auditagainst the workspace lockfile, failing the build on any vulnerability advisory; also add a scheduled trigger (e.g. weekly cron) so advisories are caught between pushes. (2) Acontracttype-sizejob (or extra steps in the existingtestjob) that runs the new encoding-size tests for Lease and Subscription so any change to their #[contracttype] serialized size fails CI. Keep the existing jobs unchanged. (.github/workflows/ci.yml)Leasevalue via Soroban XDR (soroban_sdk::xdr::ToXdr/env.to_xdr) and asserts its encoded byte length equals a committed golden value, so any change to theLease#[contracttype] layout or field sizes is caught. Include a comment explaining that the golden value must be updated deliberately when the type changes, and that CI runs this test.; (2) Add tests asserting the numeric discriminants of theErrorenum (e.g.assert_eq!(Error::AlreadyInitialised as u32, 7)) so the lease's error-code numbering is pinned and any renumbering is intentional. Add a doc comment on theErrorenum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/access-lease/src/lib.rs)Subscriptionvalue via Soroban XDR and asserts its encoded byte length equals a committed golden value, so any change to theSubscription#[contracttype] layout or field sizes is caught. Include a comment explaining that the golden value must be updated deliberately when the type changes, and that CI runs this test.; (2) Add tests asserting the numeric discriminants of theErrorenum (e.g.assert_eq!(Error::AlreadyInitialised as u32, 9)) so the subscription's error-code numbering is pinned and any renumbering is intentional. Add a doc comment on theErrorenum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/subscription/src/lib.rs)Errorenum (e.g.assert_eq!(Error::AlreadyInitialised as u32, 8)) so the registry's error-code numbering is pinned and any renumbering is intentional. Add a doc comment on theErrorenum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/vault-registry/src/lib.rs)AlreadyInitialised= 8 in vault-registry, 7 in access-lease, 9 in subscription — plusNotInitialised,NotAdmin, etc. Reference this table from the Common conventions 'Errors' row and from each contract's Errors section. (docs/CONTRACTS.md)set_adminrow to the access-control matrix in section 2 documenting that the required signer is the current admin (and, for access-lease, that the two-steppropose_admin/accept_adminflow requires the pending admin to signaccept_admin), with the non-signer test name (e.g.set_admin_by_non_admin_is_rejected). This makes the matrix consistent with SV-7 being marked Resolved. (SECURITY_AUDIT.md)