diff --git a/bettapay_common/src/storage.rs b/bettapay_common/src/storage.rs index 32ea3097..efc61a14 100644 --- a/bettapay_common/src/storage.rs +++ b/bettapay_common/src/storage.rs @@ -3,18 +3,33 @@ //! Each contract keeps its own private `DataKey` enum for keys that are //! contract-specific (e.g. settlement's `Merchant(Address)` or governance's //! `Anchor(Address)`). The keys that are semantically shared — the pause -//! flag, the recovery address, and the pending recovery operation — live in -//! [`CommonDataKey`] so every contract reads and writes them in exactly the -//! same shape. The admin role is *not* one of these: both contracts store it -//! as a multisig `Vec
` under their own `DataKey::Admin`, so there is -//! no single-`Address` shape for this crate to own. +//! flag, the recovery address, and the pending recovery operation — are +//! declared exactly once, in [`CommonDataKey`] below, and both contracts +//! import that type rather than redeclaring their own `Paused` / +//! `RecoveryAddress` / `PendingRecovery` variants. That single declaration +//! is what "storage separation" means here: `governance_contract` and +//! `settlement_contract` each have their own instance-storage ledger entry +//! (Soroban storage is scoped per contract instance), so sharing this enum +//! does not share any *data* between them — a settlement contract's +//! `CommonDataKey::Paused` and a governance contract's +//! `CommonDataKey::Paused` are two independent booleans in two independent +//! ledger entries. The type is shared purely so both contracts agree on the +//! wire shape of these keys; nothing here lets one contract read or write +//! the other's storage. The admin role is *not* one of these: both +//! contracts store it as a multisig `Vec
` under their own +//! `DataKey::Admin`, so there is no single-`Address` shape for this crate +//! to own. //! -//! The on-chain SCVal encoding of a Soroban `#[contracttype]` enum is based on -//! the variant name only; the parent enum's Rust name is not part of the -//! encoding. So a value written under `governance_contract::DataKey::Paused` -//! reads back identically through `bettapay_common::CommonDataKey::Paused`, -//! which is what allows both contracts to share this enum without disturbing -//! any existing storage entry. +//! Historical note: `Paused` (and `RecoveryAddress` / `PendingRecovery`) +//! used to be declared redundantly in each contract's own `DataKey` enum +//! as well as here. That was safe only because the on-chain SCVal encoding +//! of a Soroban `#[contracttype]` enum is based on the variant name alone — +//! the parent enum's Rust name is not part of the encoding — so a value +//! written under the old `governance_contract::DataKey::Paused` read back +//! identically through `CommonDataKey::Paused`. The duplicate variants have +//! since been removed from both contracts' `DataKey` enums; `CommonDataKey` +//! is now the single source for these keys, and no storage migration was +//! needed to get there. use soroban_sdk::{contracttype, Address, Env, String, Vec}; @@ -126,7 +141,55 @@ pub fn bump_instance_ttl(env: &Env) { #[cfg(test)] mod tests { use super::*; - use soroban_sdk::{testutils::Address as _, vec}; + use soroban_sdk::{contract, testutils::Address as _, vec}; + + /// No-op contract used only to obtain a real, registered contract + /// address for [`Env::as_contract`] — instance storage helpers like + /// [`is_paused`]/[`set_paused`] can only be exercised inside a + /// registered contract's storage context. + #[contract] + struct DummyContract; + + #[test] + fn is_paused_defaults_to_false_and_round_trips_via_common_data_key() { + let env = Env::default(); + let contract_id = env.register_contract(None, DummyContract); + env.as_contract(&contract_id, || { + // No entry written yet — CommonDataKey::Paused is the *only* + // pause key either contract can read, so a missing entry must + // read back as unpaused rather than panicking. + assert!(!is_paused(&env)); + + set_paused(&env, true); + assert!(is_paused(&env)); + + set_paused(&env, false); + assert!(!is_paused(&env)); + }); + } + + #[test] + fn two_contract_instances_have_independent_paused_flags() { + // Instance storage is scoped per contract address, so pausing one + // instance under CommonDataKey::Paused must not affect another + // instance that happens to share the same key type — this is what + // "storage separation" means despite governance_contract and + // settlement_contract both using CommonDataKey. + let env = Env::default(); + let governance_like = env.register_contract(None, DummyContract); + let settlement_like = env.register_contract(None, DummyContract); + + env.as_contract(&governance_like, || { + set_paused(&env, true); + }); + + env.as_contract(&settlement_like, || { + assert!(!is_paused(&env)); + }); + env.as_contract(&governance_like, || { + assert!(is_paused(&env)); + }); + } #[test] fn primary_admin_returns_first_entry() { diff --git a/bettapay_common/test_snapshots/storage/tests/is_paused_defaults_to_false_and_round_trips_via_common_data_key.1.json b/bettapay_common/test_snapshots/storage/tests/is_paused_defaults_to_false_and_round_trips_via_common_data_key.1.json new file mode 100644 index 00000000..df8bbcfa --- /dev/null +++ b/bettapay_common/test_snapshots/storage/tests/is_paused_defaults_to_false_and_round_trips_via_common_data_key.1.json @@ -0,0 +1,88 @@ +{ + "generators": { + "address": 1, + "nonce": 0 + }, + "auth": [ + [] + ], + "ledger": { + "protocol_version": 21, + "sequence_number": 0, + "timestamp": 0, + "network_id": "0000000000000000000000000000000000000000000000000000000000000000", + "base_reserve": 0, + "min_persistent_entry_ttl": 4096, + "min_temp_entry_ttl": 16, + "max_entry_ttl": 6312000, + "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": [ + { + "key": { + "vec": [ + { + "symbol": "Paused" + } + ] + }, + "val": { + "bool": false + } + } + ] + } + } + } + }, + "ext": "v0" + }, + 518400 + ] + ], + [ + { + "contract_code": { + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_code": { + "ext": "v0", + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "code": "" + } + }, + "ext": "v0" + }, + 518400 + ] + ] + ] + }, + "events": [] +} \ No newline at end of file diff --git a/bettapay_common/test_snapshots/storage/tests/two_contract_instances_have_independent_paused_flags.1.json b/bettapay_common/test_snapshots/storage/tests/two_contract_instances_have_independent_paused_flags.1.json new file mode 100644 index 00000000..e872e9ce --- /dev/null +++ b/bettapay_common/test_snapshots/storage/tests/two_contract_instances_have_independent_paused_flags.1.json @@ -0,0 +1,122 @@ +{ + "generators": { + "address": 2, + "nonce": 0 + }, + "auth": [ + [], + [], + [] + ], + "ledger": { + "protocol_version": 21, + "sequence_number": 0, + "timestamp": 0, + "network_id": "0000000000000000000000000000000000000000000000000000000000000000", + "base_reserve": 0, + "min_persistent_entry_ttl": 4096, + "min_temp_entry_ttl": 16, + "max_entry_ttl": 6312000, + "ledger_entries": [ + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": [ + { + "key": { + "vec": [ + { + "symbol": "Paused" + } + ] + }, + "val": { + "bool": true + } + } + ] + } + } + } + }, + "ext": "v0" + }, + 518400 + ] + ], + [ + { + "contract_data": { + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": "ledger_key_contract_instance", + "durability": "persistent" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_data": { + "ext": "v0", + "contract": "CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFCT4", + "key": "ledger_key_contract_instance", + "durability": "persistent", + "val": { + "contract_instance": { + "executable": { + "wasm": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + }, + "storage": null + } + } + } + }, + "ext": "v0" + }, + 518400 + ] + ], + [ + { + "contract_code": { + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + } + }, + [ + { + "last_modified_ledger_seq": 0, + "data": { + "contract_code": { + "ext": "v0", + "hash": "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855", + "code": "" + } + }, + "ext": "v0" + }, + 518400 + ] + ] + ] + }, + "events": [] +} \ No newline at end of file diff --git a/governance_contract/src/lib.rs b/governance_contract/src/lib.rs index cf01e9b3..0196e438 100644 --- a/governance_contract/src/lib.rs +++ b/governance_contract/src/lib.rs @@ -202,10 +202,16 @@ const SYSTEM_PARAM_TTL_BUMP: u32 = TTL_BUMP_LEDGERS; const READ_INSTANCE_TTL_THRESHOLD: u32 = 50_000; const READ_INSTANCE_TTL_BUMP: u32 = 100_000; -// Admin, RecoveryAddress, PendingRecovery, and Paused live in +// RecoveryAddress, PendingRecovery, and Paused live in // `bettapay_common::storage::CommonDataKey` instead of here - see that // type's doc comment for why a shared key type is safe to mix with this -// contract's own storage without a migration. +// contract's own storage without a migration. Admin is NOT one of these: +// both contracts keep it as their own `DataKey::Admin` below, since it's a +// multisig `Vec
` rather than the single-`Address` shape +// `CommonDataKey` would need to own it. Storage stays fully separate +// otherwise - this contract's `DataKey` and `CommonDataKey` share no +// on-chain state, they simply both read/write the same instance-storage +// ledger entry via distinct key variants. #[derive(Clone)] #[contracttype] enum DataKey { @@ -230,9 +236,6 @@ enum DataKey { /// Storage key for the anchor address associated with a specific asset. Anchor(Address), - /// Storage key for the pause state flag. - Paused, - /// Storage key for a scheduled operation. /// Uses persistent storage, keyed by operation hash, to store execution timestamp. ScheduledOperation(BytesN<32>), diff --git a/settlement_contract/src/types.rs b/settlement_contract/src/types.rs index 5ac7a651..bd09ac0c 100644 --- a/settlement_contract/src/types.rs +++ b/settlement_contract/src/types.rs @@ -144,10 +144,16 @@ pub struct FeeConfig { pub network_fee_bps: u32, } -// Admin, RecoveryAddress, PendingRecovery, and Paused live in +// RecoveryAddress, PendingRecovery, and Paused live in // `bettapay_common::storage::CommonDataKey` instead of here - see that // type's doc comment for why a shared key type is safe to mix with this -// contract's own storage without a migration. +// contract's own storage without a migration. Admin is NOT one of these: +// both contracts keep it as their own `DataKey::Admin` below, since it's a +// multisig `Vec
` rather than the single-`Address` shape +// `CommonDataKey` would need to own it. Storage stays fully separate +// otherwise - this contract's `DataKey` and `CommonDataKey` share no +// on-chain state, they simply both read/write the same instance-storage +// ledger entry via distinct key variants. #[derive(Clone)] #[contracttype] @@ -189,8 +195,6 @@ pub(crate) enum DataKey { DefaultRule, /// Persistent — one per payment, high volume. Payment(BytesN<32>), - /// Instance — singleton boolean, read on every mutating call. - Paused, /// Storage key for a scheduled operation. ScheduledOperation(BytesN<32>), }