This document outlines security best practices and checklist items for Callora vault contracts to improve audit readiness and reviewer confidence.
- All privileged functions protected by
require_auth()orrequire_auth_for_args()viaAddress - Admin state stored securely (e.g., using
env.storage().instance()) - Admin rotation/transfer tested and documented
- No integer overflow/underflow possible
- Solidity ^0.8.x overflow checks relied upon or SafeMath used where required
- For Soroban/Rust:
checked_add/checked_subused for all balance mutations -
overflow-checksenabled in both dev and release profiles
All balance mutations in
callora-vault(deposit,deduct,batch_deduct,withdraw,withdraw_to) andcallora-revenue-pool(batch_distribute) usechecked_add/checked_suband panic with a descriptive message on overflow.callora-settlement(receive_payment) does the same. The workspaceCargo.tomlsetsoverflow-checks = truefor bothdevandreleaseprofiles, so even plain arithmetic would trap in debug builds — the explicit checked calls make the intent clear and guarantee the same behaviour in all build configurations.
Additional hardening note:
- Removed a duplicated
get_max_deductentrypoint declaration incallora-vaultto avoid ambiguous review surfaces and keep ABI-facing code paths singular. The function is retained as a private internal helper called bydeductandbatch_deduct.
-
initializefunction protected against multiple calls (e.g., checking if admin key exists ininstance()storage) - Contract upgrades (
env.deployer().update_current_contract_wasm()) protected byrequire_auth() - No unprotected re-init functions
-
initializevalidates all input parameters
- Emergency pause mechanism implemented via state flag in
instance()storage - Paused state blocks fund movement (e.g., reverting via
panic_with_error!) - Pause/unpause flows tested
-
is_paused()view function exposed for off-chain monitoring - View function is read-only, deterministic, and non-panicking
- Safe default state (returns
falsewhen unset)
- Ownership transfer is two-step (optional but recommended)
- Ownership transfer emits events
- Renounce ownership reviewed and justified
The vault exposes a dedicated authorized_caller role (stored in VaultMeta
and settable via set_authorized_caller) that is permitted to invoke
balance-mutating operations such as deduct and batch_deduct. This role is
distinct from owner and admin, and reviewers should confirm the following
controls are in place:
-
authorized_calleris stored inVaultMetaunder theMetainstance storage key and is not duplicated in any other location - Only the current
ownercan set or rotateauthorized_callerviaset_authorized_caller(enforced bymeta.owner.require_auth()) -
set_authorized_calleremits aset_authorized_callerevent with the owner as topic and(old_authorized_caller, new_authorized_caller)as data, enabling off-chain monitoring of role changes and clear audit diffs during rotation -
deductandbatch_deductreject callers that are not the currently configuredauthorized_caller(panic:unauthorized: caller is not the authorized caller) - When
authorized_callerisNone, deduct-class operations fall back to owner-only execution; non-owner callers remain rejected - Rotation flow (set → use → rotate → old caller rejected) covered by
unit tests in
contracts/vault/src/test.rs - Role changes are reviewed as part of the operational runbook; the new
caller address is verified off-chain (e.g. multisig or governance) before
the owner signs
set_authorized_caller -
authorized_calleris scoped strictly to deduct-class operations and does not grant the ability to withdraw, distribute, pause, or upgrade the contract
Security note:
authorized_calleris intentionally a narrow-privilege role meant for the off-chain billing/settlement driver. It can spend vault balance viadeduct/batch_deductwithin the configuredmax_deductlimit, so the owning key should rotate it immediately if the off-chain driver's signing key is suspected of compromise. Because rotation is a single-call owner-only operation with an emitted event, recovery is observable and atomic.
-
deductandbatch_deducttreatrequest_idas a single-use idempotency key when it is provided - Duplicate
request_idvalues are rejected before any balance mutation, transfer, or event emission - Batch validation rejects both replayed request ids from prior calls and duplicate ids repeated inside the same batch
- Unit tests cover duplicate single-call replay and duplicate-in-batch rejection with atomic balance assertions
- Token transfers strictly rely on
soroban_sdk::token::Client - Cross-contract calls handle potential errors/panics gracefully
- State changes are persisted before making cross-contract calls to mitigate subtle state-caching issues
- Checks-effects-interactions pattern followed
The vault performs USDC transfers to configurable counterpart addresses on every
deduct and batch_deduct call. These external transfers are justified as follows:
- settlement address: set and updated exclusively by the on-chain admin via
set_settlement. This function emits aset_settlementevent to provide a clear audit trail for address rotation. Transfers to this address implement the documentedVault → Settlementrevenue flow described inSETTLEMENT_IMPLEMENTATION.md. - revenue_pool address: retained as an informational configuration slot via
set_revenue_pool/get_revenue_pool. It is no longer consulted during deducts —deductandbatch_deductalways route to the settlement address. - CRITICAL — Settlement Required (Issue #263):
deductandbatch_deductpanic with"settlement address not set"whenset_settlementhas not been called. The panic occurs before any balance mutation or event emission, so the transaction reverts atomically with no observable state change. This closes the silent-loss-of-accounting window where the internalbalancecould previously decrement without a corresponding on-ledger USDC transfer. - Address Validation: Both
set_settlement()andset_revenue_pool()validate that the provided address is NOT the vault's own address, preventing self-referential routing loops. - Atomic Updates: Each address is updated atomically in a single storage write, ensuring no partial update is observable by other callers.
- Audit Trail: All routing configuration changes emit events:
set_settlement(admin) → addresswhen setting settlementset_revenue_pool(admin) → addresswhen setting revenue poolclear_revenue_pool(admin) → ()when clearing revenue pool
- Deposit/withdraw invariants tested
- Vault balance accounting verified
- Funds cannot be locked permanently
- Minimum deposit requirements enforced
- Maximum deduction limits enforced (
get_max_deduct/set_max_deduct) with explicit positive-value validation and dedicated unit tests. - Revenue pool transfers validated
- Settlement developer address required when routing to specific developer.
- Settlement developer address must be None when routing to global pool.
- Batch operations respect individual limits
The Revenue Pool contract (contracts/revenue_pool) operates under the following security assumptions and threat models:
-
Malicious Admin: The
adminrole has the authority to distribute funds and replace the admin address. A compromised or malicious admin could drain the pool's USDC balance.- Mitigation: The
adminshould always be a heavily guarded multisig account or a rigorously audited governance contract.
- Mitigation: The
-
Wrong USDC Token Initialization: The
usdc_tokenaddress is set once duringinit. If initialized with a malicious or incorrect token address, the pool will process the wrong asset.- Mitigation: The deployment process must verify the official Stellar USDC (or appropriate wrapped USDC) contract address before initialization. The
initfunction guards against re-initialization.
- Mitigation: The deployment process must verify the official Stellar USDC (or appropriate wrapped USDC) contract address before initialization. The
-
Operational Griefing (Balances): Anyone can effectively transfer USDC to the revenue pool. If an attacker sends unsolicited funds, it increases the
balance()but does not disrupt thedistributelogic, as distribution is explicitly controlled by the admin.- Mitigation: The pool does not rely on strict balance equality invariants for its core operations, mitigating balance-based operational griefing. The
receive_paymententrypoint is admin-only and event-only (no token movement), so indexers should reconcilereceive_paymentlogs with actual token transfers.
- Mitigation: The pool does not rely on strict balance equality invariants for its core operations, mitigating balance-based operational griefing. The
-
Resource Exhaustion via Unbounded Batch:
batch_distributeaccepts aVec<(Address, i128)>. Without a cap, a compromised admin key could submit thousands of entries, exhausting Soroban's per-transaction CPU/memory budget and causing unpredictable mid-execution failures.- Mitigation:
batch_distributeenforces1 <= payments.len() <= MAX_BATCH_SIZE(currently 50), matching the vault'sbatch_deductcap. Empty vectors and oversized vectors are rejected before any iteration or USDC transfer occurs. The cap keeps resource consumption well within Soroban network limits.
- Mitigation:
- All amounts validated to be > 0
- Address/parameter validation on all public functions
- Boundary conditions tested (max values, zero values)
- Error messages provide clear context for debugging
callora-vault::initenforcesmin_deposit > 0; omitted values default to1.
- All state changes emit appropriate events
- Event schema documented and indexed
- Critical operations (deposit, withdraw, deduct) logged with full context
- Unit tests assert
depositanddeductevent topics/data (caller, request_id semantics, and resulting balance). -
callora-revenue-pool::set_adminemits an explicitadmin_changedevent carrying(old_admin, new_admin)beforeadmin_transfer_started, and unit tests pin topics/data.
- Unit tests cover all public functions
- Edge cases and boundary conditions tested
- Panic scenarios tested with
#[should_panic] - Integration tests for complete user flows
- Minimum 95% test coverage maintained (enforced via
cargo tarpaulinwithfail-under = 95.0)
Before any mainnet deployment:
-
Engage an independent third-party security auditor
- Choose auditors with experience in Soroban/Stellar smart contracts
- Ensure auditor understands vault-specific risk patterns
-
Perform a full smart contract audit
- Review all contract code for security vulnerabilities
- Analyze upgrade patterns and migration paths
- Validate mathematical correctness of balance operations
-
Address all high and medium severity findings
- Create tracking system for audit findings
- Implement fixes for all H/M severity issues
- Document rationale for any low severity findings that won't be fixed
-
Publish audit report for transparency
- Make audit report publicly available
- Include summary of findings and remediation steps
- Provide evidence of test coverage and validation
- WASM compilation verified and reproducible (
stellar contract build/cargo build --target wasm32-unknown-unknown --release) - Storage lifespan (
extend_ttl) implemented to prevent state archiving for critical data - Stellar network parameters validated (budget, CPU/RAM limits)
- Cross-contract call security and generic type usage (
Val) reviewed - Storage patterns optimized and secure (e.g., correct usage of
persistentvsinstancevstemporarykeys)
- Fee structures reviewed for economic attacks
- Revenue pool distribution validated
- Maximum loss scenarios analyzed
- Slippage and market impact considered
- Deployment process documented and automated
- Key management procedures established
- Monitoring and alerting configured
- Incident response plan prepared
- Stellar Security Best Practices
- Soroban Documentation
- Smart Contract Weakness Classification Registry
Note: This checklist should be reviewed and updated regularly as new security patterns emerge and the codebase evolves.
All privileged entrypoints across vault, revenue_pool, and settlement contracts
have been audited for require_auth() coverage as part of Issue #160.
- All privileged functions call
require_auth()on the caller before executing. ✅ - Negative tests added to each crate's
test.rsconfirming unauthenticated calls are rejected.
| Contract | Function | Reason |
|---|---|---|
| settlement | init() |
One-time initializer guarded by already-initialized panic; no auth required by design. |
| vault | require_owner() |
Internal helper using assert! for address equality. All public callers invoke caller.require_auth() before calling this helper, so host-level auth is enforced transitively. Documented gap: require_owner itself does not call require_auth(). |
- Audit branch:
test/require-auth-sweep - Tests:
contracts/vault/src/test.rs,contracts/revenue_pool/src/test.rs,contracts/settlement/src/test.rs
As part of the authorization matrix hardening for the callora-settlement contract:
get_all_developer_balancesnow requiresadminauthorization viarequire_auth(). This prevents bulk data scraping while allowing administrative oversight.- Comprehensive negative tests have been added to
contracts/settlement/src/test.rscoveringreceive_payment,set_admin,set_vault, andget_all_developer_balances. - Overflow regression tests now assert
receive_paymentpanics with"pool balance overflow"and"developer balance overflow"when credits would exceedi128::MAX. - Admin rotation (two-step) has been verified to correctly gate access during the transition period.