Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ resolver = "2"
members = ["contracts/vc-vault"]

[workspace.package]
version = "0.20.0"
version = "0.21.0"
edition = "2021"
license = "Apache-2.0"
repository = "https://github.com/ACTA-Team/contracts"

[workspace.dependencies]
soroban-sdk = { version = "21.0.0" }
soroban-sdk = { version = "23.4.0" }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

soroban-sdk 23.4.0 breaking changes from version 21 release notes changelog

💡 Result:

Breaking changes when upgrading soroban-sdk v21 → v23.4.0

v21 → v22 (breaking API changes)

  • Contract test registration API renamed/unified
    • Env::register / Env::register_at replace Env::register_contract / Env::register_contract_wasm. [1]
  • Contract deployment API changed
    • DeployerWithAddress::deploy_v2 replaces DeployerWithAddress::deploy (adds constructor-args parameter; pass () if none). [1]
  • Fuzz/panic catching API deprecated
    • fuzz_catch_panic deprecated; use Env::try_invoke_contract and generated try_ client methods. [1]
  • Test snapshot event set changed
    • Test snapshots no longer include diagnostic events—only contract + system events remain. [1]

v22 → v23 (breaking behavior / interface changes)

  • Events API migration
    • #[contractevent] replaces Events::publish (old publish path deprecated). [1]
  • Token transfer “to” type change
    • MuxedAddress replaces Address as the to argument of TokenInterface::transfer (in soroban-token-sdk, but it impacts contracts using that interface). [1]
  • Testing behavior for archived persistent entries changed
    • Accessing archived persistent entries in tests no longer panics; restoration is emulated (instance storage is also persistent). [1]

v23.0.0 → v23.4.0

  • No additional “Breaking Changes” section was called out in the v23.4.0 release notes; it’s primarily incremental updates (e.g., making trait functions with default implementations callable, test snapshot source options). [2]
  • Most of the major v23-line breakage is concentrated at v23.0.0 (muxed address support, contractevent introduction, deprecations around events). [1][3]

Sources: docs.rs migration guide [1], GitHub releases v23.4.0 [2] and v23.0.0 [3].


🌐 Web query:

soroban-sdk Address require_auth Env mock_all_auths breaking changes 22 23

💡 Result:

In the soroban-sdk v22 → v23 timeframe, the main auth-related “gotchas” behind Address::require_auth + Env::mock_all_auths test failures are:

  1. Address::require_auth() is “require_auth_for_args with implied args”
    require_auth() authorizes the current contract invocation including all invocation arguments, which are automatically inferred (it’s equivalent to calling require_auth_for_args with the invocation’s args). This matters because auth matching/mocking is sensitive to the args being authorized. (docs.rs)

  2. Env::mock_all_auths() became stricter about “root invocation” auth coverage
    In testutils, mock_all_auths() mocks all require_auth / require_auth_for_args calls, but it will error if an address is only authorized in a non-root sub-invocation and that address wasn’t require_auth’d in the root invocation. This commonly shows up when Contract B calls Contract A, and only A does require_auth. (docs.rs)

  3. New escape hatch: mock_all_auths_allowing_non_root_auth()
    To support the “auth only happens in a subcall” pattern in tests, v23-era SDKs provide Env::mock_all_auths_allowing_non_root_auth(), which is identical to mock_all_auths() except it does not fail when the root invocation didn’t require_auth for a given address. (docs.rs)

If you share the exact error/panic text and a minimal test snippet (root call + any cross-contract calls), I can point to which of the above is triggering and what to change.

Citations:


🌐 Web query:

soroban-sdk storage TTL API changes version 23 migration guide

💡 Result:

What changed in Protocol/Soroban SDK v23 around storage TTL / archival

  • Archived-entry access behavior changed (automatic restoration): In Protocol 23, InvokeHostFunction can automatically restore archived entries in the transaction footprint before contract execution, so “manual restoration” is generally no longer required. This also changes how apps should think about “expired/archived” state. [1][2]
  • Test behavior in soroban-sdk changed to match P23: In SDK 23, the test environment no longer errors when you read an entry whose TTL you’ve “expired” by advancing the ledger; instead it emulates auto-restoration, so reads may succeed where SDK 22 tests expected failure (this includes instance storage, since it is persistent too). [3]

Migration guide (v22 ➜ v23)

  1. Fix tests that expected archived reads to fail

    • If you had tests that advanced the ledger past an entry’s TTL and asserted a read fails, update them.
    • Prefer asserting the TTL value directly after your bump/extend logic (e.g., use get_ttl in testutils) rather than relying on an “archived read throws” behavior. [3]
  2. Re-check your “restore vs extend TTL” transaction flow (off-chain)

    • With P23 auto-restoration, the workflow shifts toward:
      • Preflight/simulate to build the correct footprint (including any archived entries that will be restored), then
      • Use Extend Footprint TTL when you want to keep entries alive longer.
    • In the JS SDK, ExtendFootprintTTLOp takes extendTo (a new minimum TTL applied to entries in the transaction’s read-only footprint; entries already above it are skipped). [2][4]

Sources: Protocol 23 announcement (auto-restoration / no manual restore) [1], Stellar Core P23 release notes (auto-restoration) [2], Soroban SDK v23 migration note for tests [3], JS SDK docs for ExtendFootprintTTLOp semantics [4].


Validate Soroban SDK v21 → v23 migration before merge.

The upgrade spans two major versions with confirmed breaking changes:

  • Test utilities: Env::register_contract renamed to Env::register; DeployerWithAddress::deploy replaced by deploy_v2 (requires constructor args parameter)
  • Auth testing: Env::mock_all_auths() became stricter about root invocation auth coverage; use mock_all_auths_allowing_non_root_auth() if auth only occurs in subcalls
  • Storage/TTL behavior: Test environment now emulates auto-restoration of archived entries instead of panicking on reads; update tests relying on "archived read fails" assertions
  • Events: #[contractevent] replaces Events::publish; check if contract uses old event publishing

Confirm the test suite handles these changes, particularly around auth mocking and TTL assertions.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Cargo.toml` at line 13, The Soroban SDK bump from v21→v23 introduces breaking
API and behavior changes; update tests and contract code accordingly: replace
Env::register_contract calls with Env::register, migrate any
DeployerWithAddress::deploy usage to deploy_v2 and pass the required constructor
args, switch Env::mock_all_auths() to mock_all_auths_allowing_non_root_auth()
for tests that only authenticate in subcalls, adjust TTL/storage tests to expect
auto-restoration of archived entries instead of panics, and replace any
Events::publish-based event tests/usage with the new #[contractevent]-based
publishing API; run the test suite and fix any resulting failures referencing
these symbols (Env::register, deploy_v2, mock_all_auths_allowing_non_root_auth,
#[contractevent], archived/TTL assertions).


[profile.release]
opt-level = "z"
Expand All @@ -22,6 +22,7 @@ panic = "abort"
codegen-units = 1
lto = true

# For more information about this profile see https://soroban.stellar.org/docs/basic-tutorials/logging#cargotoml-profile
[profile.release-with-logs]
inherits = "release"
debug-assertions = true
5 changes: 5 additions & 0 deletions contracts/vc-vault/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,9 @@ pub trait VcVaultTrait {
) -> String;
fn revoke(e: Env, vc_id: String, date: String);
fn migrate(e: Env, owner: Option<Address>);
fn create_sponsored_vault(e: Env, sponsor: Address, owner: Address, did_uri: String);
fn set_sponsored_vault_open_to_all(e: Env, open: bool);
fn get_sponsored_vault_open_to_all(e: Env) -> bool;
fn add_sponsored_vault_sponsor(e: Env, sponsor: Address);
fn remove_sponsored_vault_sponsor(e: Env, sponsor: Address);
}
56 changes: 56 additions & 0 deletions contracts/vc-vault/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,62 @@ impl VcVaultTrait for VcVaultContract {
storage::extend_vc_status_ttl(&e, &vc_id);
}

// --- Sponsored vault ---

/// Create a vault on behalf of `owner`. `sponsor` must sign.
/// If restricted (open_to_all = false): sponsor must be contract admin or in sponsors list.
/// If open (open_to_all = true): any signer can be sponsor.
fn create_sponsored_vault(e: Env, sponsor: Address, owner: Address, did_uri: String) {
sponsor.require_auth();
if !storage::has_contract_admin(&e) {
panic_with_error!(e, ContractError::NotInitialized);
}
if !storage::read_sponsored_vault_open_to_all(&e) {
let admin = storage::read_contract_admin(&e);
if sponsor != admin && !storage::is_authorized_sponsor(&e, &sponsor) {
panic_with_error!(e, ContractError::NotAuthorizedSponsor);
}
}
if storage::has_vault_admin(&e, &owner) {
panic_with_error!(e, ContractError::AlreadyInitialized);
}
storage::write_vault_admin(&e, &owner, &owner);
storage::write_vault_did(&e, &owner, &did_uri);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity severity

Access Control and Authorization: Unconsented vault initialization for owner in create_sponsored_vault

create_sponsored_vault only requires sponsor.require_auth() and then writes vault state for an arbitrary owner, including VaultDid. In open_to_all mode this allows any signer to pre-create (squat) a vault for any target address and permanently set arbitrary DID metadata, blocking the target from calling create_vault later (AlreadyInitialized) and potentially poisoning off-chain identity assumptions since there is no DID-update entrypoint.

Require explicit owner consent (e.g., owner.require_auth() or a signed owner authorization payload) OR make did_uri mutable by the vault admin so the owner can correct sponsored initialization; additionally consider restricting open_to_all to owner == sponsor if you want "self-sponsoring" only.


Actions
  • Reply /almanax ask <question> to ask a follow-up question.
  • Reply /almanax dismiss [<reason>] and it won't appear again in future scans.
  • Reply /almanax resolve [<reason>] to mark the finding as resolved.
  • Reply /almanax severity <severity> [<reason>] to override the severity.

storage::write_vault_revoked(&e, &owner, &false);
storage::write_vault_issuers(&e, &owner, &Vec::new(&e));
storage::extend_vault_ttl(&e, &owner);
storage::extend_instance_ttl(&e);
}

/// Enable or disable the sponsor restriction. Admin only.
/// open = false (default): only admin or sponsors list can create sponsored vaults.
/// open = true: anyone can create sponsored vaults.
fn set_sponsored_vault_open_to_all(e: Env, open: bool) {
validate_contract_admin(&e);
storage::write_sponsored_vault_open_to_all(&e, &open);
storage::extend_instance_ttl(&e);
}

/// Query whether sponsored vault creation is open to all.
fn get_sponsored_vault_open_to_all(e: Env) -> bool {
storage::extend_instance_ttl(&e);
storage::read_sponsored_vault_open_to_all(&e)
}

/// Add an address to the authorized sponsors list. Admin only.
fn add_sponsored_vault_sponsor(e: Env, sponsor: Address) {
validate_contract_admin(&e);
storage::add_sponsored_vault_sponsor(&e, &sponsor);
storage::extend_instance_ttl(&e);
}

/// Remove an address from the authorized sponsors list. Admin only.
fn remove_sponsored_vault_sponsor(e: Env, sponsor: Address) {
validate_contract_admin(&e);
storage::remove_sponsored_vault_sponsor(&e, &sponsor);
storage::extend_instance_ttl(&e);
}

// --- Migrations ---

/// Migrate legacy storage. Some(owner) = vault migration; None = issuance registry migration.
Expand Down
2 changes: 2 additions & 0 deletions contracts/vc-vault/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ pub enum ContractError {
NotInitialized = 9,
/// vault_contract param is not this contract.
InvalidVaultContract = 10,
/// Signer is not the contract admin nor an authorized sponsor.
NotAuthorizedSponsor = 11,
}
50 changes: 50 additions & 0 deletions contracts/vc-vault/src/storage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ pub enum DataKey {
LegacyIssuanceRevocations,
LegacyIssuanceVCs,
LegacyVaultVCs(Address),
SponsoredVaultOpenToAll,
SponsoredVaultSponsors,
}

/// Legacy revocation record for migration.
Expand Down Expand Up @@ -393,6 +395,54 @@ pub fn extend_vc_status_ttl(e: &Env, vc_id: &String) {
}
}

// --- Sponsored vault config (instance) ---

pub fn read_sponsored_vault_open_to_all(e: &Env) -> bool {
e.storage()
.instance()
.get(&DataKey::SponsoredVaultOpenToAll)
.unwrap_or(false)
}

pub fn write_sponsored_vault_open_to_all(e: &Env, open: &bool) {
e.storage()
.instance()
.set(&DataKey::SponsoredVaultOpenToAll, open);
}

pub fn read_sponsored_vault_sponsors(e: &Env) -> Vec<Address> {
e.storage()
.instance()
.get(&DataKey::SponsoredVaultSponsors)
.unwrap_or_else(|| Vec::new(e))
}

pub fn write_sponsored_vault_sponsors(e: &Env, sponsors: &Vec<Address>) {
e.storage()
.instance()
.set(&DataKey::SponsoredVaultSponsors, sponsors);
}

pub fn is_authorized_sponsor(e: &Env, sponsor: &Address) -> bool {
read_sponsored_vault_sponsors(e).contains(sponsor.clone())
}

pub fn add_sponsored_vault_sponsor(e: &Env, sponsor: &Address) {
let mut sponsors = read_sponsored_vault_sponsors(e);
if !sponsors.contains(sponsor.clone()) {
sponsors.push_front(sponsor.clone());
write_sponsored_vault_sponsors(e, &sponsors);
}
}

pub fn remove_sponsored_vault_sponsor(e: &Env, sponsor: &Address) {
let mut sponsors = read_sponsored_vault_sponsors(e);
if let Some(idx) = sponsors.first_index_of(sponsor.clone()) {
sponsors.remove(idx);
write_sponsored_vault_sponsors(e, &sponsors);
}
}

// --- Legacy (migration) ---

pub fn read_legacy_issuance_vcs(e: &Env) -> Option<Vec<String>> {
Expand Down
97 changes: 97 additions & 0 deletions contracts/vc-vault/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,3 +430,100 @@ fn test_migrate_some_without_legacy_vault_panics() {
client.create_vault(&owner, &String::from_str(&env, "did:pkh:stellar:testnet:OWNER"));
client.migrate(&Some(owner));
}

// --- Sponsored vault tests ---

#[test]
fn test_sponsored_vault_open_to_all_defaults_false() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
assert!(!client.get_sponsored_vault_open_to_all());
}

#[test]
fn test_admin_creates_sponsored_vault() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&admin, &owner, &did_uri);
// Vault exists: list_vc_ids returns empty without panicking.
assert_eq!(client.list_vc_ids(&owner).len(), 0);
}
Comment on lines +444 to +452
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

list_vc_ids is a weak proxy for “vault was created.”

These assertions can pass even if vault creation silently failed, because list_vc_ids can return an empty list for missing vault state as well.

💡 Suggested test assertion hardening
-    // Vault exists: list_vc_ids returns empty without panicking.
-    assert_eq!(client.list_vc_ids(&owner).len(), 0);
+    // Proves vault initialization path succeeded.
+    let new_admin = Address::generate(&env);
+    client.set_vault_admin(&owner, &new_admin);

Also applies to: 455-464, 480-490

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@contracts/vc-vault/src/test.rs` around lines 444 - 452, The test currently
uses client.list_vc_ids(&owner) to infer vault creation which can be true even
if vault creation silently failed; update the test_admin_creates_sponsored_vault
(and the other affected tests at the noted ranges) to assert vault creation
explicitly by checking the vault state or creation result: verify the return
value or success of client.create_sponsored_vault, then call a direct
existence/state accessor such as client.get_vault(&owner) or
client.vault_exists(&owner) (or inspect vault metadata/owner mapping exposed by
the contract) and assert the expected values (e.g., non-None result or matching
owner/did_uri) instead of relying on list_vc_ids being empty.


#[test]
fn test_authorized_sponsor_creates_sponsored_vault() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
let sponsor = Address::generate(&env);
client.add_sponsored_vault_sponsor(&sponsor);
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&sponsor, &owner, &did_uri);
assert_eq!(client.list_vc_ids(&owner).len(), 0);
}

#[test]
#[should_panic]
fn test_unauthorized_address_cannot_create_sponsored_vault_in_restricted_mode() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
// Confirm restricted mode (default).
assert!(!client.get_sponsored_vault_open_to_all());
let random = Address::generate(&env);
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&random, &owner, &did_uri);
}

#[test]
fn test_open_mode_allows_anyone_to_create_sponsored_vault() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
client.set_sponsored_vault_open_to_all(&true);
assert!(client.get_sponsored_vault_open_to_all());
let random = Address::generate(&env);
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&random, &owner, &did_uri);
assert_eq!(client.list_vc_ids(&owner).len(), 0);
}

#[test]
#[should_panic]
fn test_back_to_restricted_mode_blocks_unauthorized() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
client.set_sponsored_vault_open_to_all(&true);
client.set_sponsored_vault_open_to_all(&false);
let random = Address::generate(&env);
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&random, &owner, &did_uri);
}

#[test]
#[should_panic]
fn test_removed_sponsor_cannot_create_sponsored_vault() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
let sponsor = Address::generate(&env);
client.add_sponsored_vault_sponsor(&sponsor);
client.remove_sponsored_vault_sponsor(&sponsor);
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
// Must fail: sponsor was removed.
client.create_sponsored_vault(&sponsor, &owner, &did_uri);
}

#[test]
#[should_panic]
fn test_duplicate_sponsored_vault_panics() {
let (env, admin, _issuer, _contract_id, client) = setup();
client.initialize(&admin, &String::from_str(&env, "did:acta:default"));
let owner = Address::generate(&env);
let did_uri = String::from_str(&env, "did:pkh:stellar:testnet:OWNER");
client.create_sponsored_vault(&admin, &owner, &did_uri);
// Second creation for same owner must fail.
client.create_sponsored_vault(&admin, &owner, &did_uri);
}