Skip to content

Fix: 4 issues (#107, #109, #112, #113) - #150

Merged
Busiii-adetiba merged 2 commits into
SynapsVault:mainfrom
k2ghostyou:simple/107-109-112-113
Oct 18, 2026
Merged

Busiii-adetiba merged 2 commits into
SynapsVault:mainfrom
k2ghostyou:simple/107-109-112-113

Conversation

@k2ghostyou

Copy link
Copy Markdown
Contributor

Closes #107
Closes #109
Closes #112
Closes #113

Issues

#107 [cross-cutting] Add cargo audit (or equivalent dependency audit) to CI

Found in review of the contracts (item 75 of 93).

Type: enhancement
Area: cross-cutting (59–80)

Add cargo audit (or equivalent dependency audit) to CI

Source: proposed-issue list from the code review of vault-registry, access-lease and subscription.

#109 [cross-cutting] Add #[contracttype] size checks to CI so Lease and Subscription encoding changes are caught

Found in review of the contracts (item 77 of 93).

Type: enhancement
Area: cross-cutting (59–80)

Add #[contracttype] size checks to CI so Lease and Subscription encoding changes are caught

Source: proposed-issue list from the code review of vault-registry, access-lease and subscription.

#112 [cross-cutting] Error code numbering differs between contracts (AlreadyInitialised is 8 in registry, 7 in lease, 9 in subsc...

Found in review of the contracts (item 80 of 93).

Type: cleanup
Area: cross-cutting (59–80)

Error code numbering differs between contracts (AlreadyInitialised is 8 in registry, 7 in lease, 9 in subscription) — document as intentional or align

Source: proposed-issue list from the code review of vault-registry, access-lease and subscription.

#113 [docs and tooling] SECURITY_AUDIT.md lists SV-7 (admin rotation) as resolved, but the access-control matrix has no row for set...

Found in review of the contracts (item 81 of 93).

Type: docs
Area: docs and tooling (81–93)

SECURITY_AUDIT.md lists SV-7 (admin rotation) as resolved, but the access-control matrix has no row for set_admin

Source: proposed-issue list from the code review of vault-registry, access-lease and subscription.

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

  • Add two new CI jobs to the workflow. (1) A cargo-audit job that installs Rust stable, installs cargo-audit (e.g. cargo install cargo-audit --locked), and runs cargo audit against 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) A contracttype-size job (or extra steps in the existing test job) 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)
  • (1) Add a test module (or extend the existing tests) that serializes a representative Lease value 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 the Lease #[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 the Error enum (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 the Error enum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/access-lease/src/lib.rs)
  • (1) Add a test module (or extend the existing tests) that serializes a representative Subscription value via Soroban XDR and asserts its encoded byte length equals a committed golden value, so any change to the Subscription #[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 the Error enum (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 the Error enum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/subscription/src/lib.rs)
  • Add tests asserting the numeric discriminants of the Error enum (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 the Error enum noting that numbering is per-contract and that the cross-contract mapping is documented in docs/CONTRACTS.md. (contracts/vault-registry/src/lib.rs)
  • Add a documented cross-contract error-code mapping table explaining that error numbering is intentionally per-contract (each enum is independent and stable, never renumbered) and listing the shared concepts with their differing codes — notably AlreadyInitialised = 8 in vault-registry, 7 in access-lease, 9 in subscription — plus NotInitialised, NotAdmin, etc. Reference this table from the Common conventions 'Errors' row and from each contract's Errors section. (docs/CONTRACTS.md)
  • Add a set_admin row to the access-control matrix in section 2 documenting that the required signer is the current admin (and, for access-lease, that the two-step propose_admin/accept_admin flow requires the pending admin to sign accept_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)

Co-authored-by: Simple Agent <agent@simple.dev>
@drips-wave

drips-wave Bot commented Oct 18, 2026

Copy link
Copy Markdown

@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! 🚀

Learn more about application limits

@Busiii-adetiba
Busiii-adetiba merged commit bae6cdd into SynapsVault:main Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment