Skip to content

fix(contract): issues 816-819 — pause expiry getter, day-index cap, t… - #960

Open
talktosam2003 wants to merge 7 commits into
SiLioLabs:masterfrom
talktosam2003:fix/issues-816-817-818-819
Open

fix(contract): issues 816-819 — pause expiry getter, day-index cap, t…#960
talktosam2003 wants to merge 7 commits into
SiLioLabs:masterfrom
talktosam2003:fix/issues-816-817-818-819

Conversation

@talktosam2003

Copy link
Copy Markdown
Contributor

Close #816
Close #817
close #818
close #819

feat(contract): API expansion, day-index safety caps, SAC token validation, and typed errors (#816, #817, #818, #819)

Title

feat(contract): implement pause expiry API, merchant day cap, SAC token validation, and typed errors


📌 Summary & Context

This pull request resolves four contract-level stability, state integrity, and developer-experience issues (#816, #817, #818, and #819).

Together, these changes:

  1. Expose missing on-chain pause metadata to off-chain keepers and clients.
  2. Prevent unbounded storage growth for high-volume merchant analytics.
  3. Fail early on invalid token inputs during subscription setup to prevent corrupt/unusable state.
  4. Convert remaining string-panic paths into deterministic, typed ContractError variants for robust client-side handling.

🛠 Detailed Breakdown of Changes

1. Issue #816 — Pause Expiry Read API & Event Consistency

  • Public View Function: Added pub fn get_pause_expiry(env: Env, user: Address) -> Option<u64> in lib.rs, which delegates directly to storage::get_pause_expiry. As a read-only view, it requires no authentication.
  • State Representation:
    • pause() writes u64::MAX to represent an indefinite pause.
    • pause_until(expiry) writes the specific Unix timestamp.
    • The getter returns Some(u64) for both states and None when active.
  • Auto-Resume Audit: Confirmed that both charge() and batch_charge() paths cleanly erase PauseExpiry from storage and emit subscription_auto_resumed events when charging past an expired pause window.

2. Issue #817 — Max Merchant Revenue Day-Index Size Cap

  • Constant & Documentation: Defined MAX_MERCHANT_REVENUE_DAY_INDEX_SIZE = 365 in merchant_stats.rs. Included inline documentation explaining that a 1-year daily index bounds storage pressure and requires operators to utilize existing prune_merchant_revenue_days APIs.
  • Append Guard: Enforced the index cap inside increment_revenue_with_daily. Attempting to append a new day beyond 365 returns ContractError::MerchantDayIndexFull (Error Code 41).
  • Update Granularity: Updating an existing day’s revenue bucket (is_new_day == false) bypasses the check, ensuring operational continuity even when the index array is full.
  • Error Definition: Registered MerchantDayIndexFull = 41 in errors.rs.

3. Issue #818 — Token Address & SAC Interface Validation

  • Validation Pipeline: Implemented require_valid_token_address() in validation.rs featuring a two-layer validation strategy:
    1. Address Type Check: Performs an XDR byte-7 discriminant check to reject standard Account addresses upfront with ContractError::InvalidTokenAddress (Error Code 12).
    2. Interface Probe: Executes a lightweight token::Client::new(env, addr).decimals() host call to verify Stellar Asset Contract (SAC) compliance without blowing CPU limits.
  • Subscription Protection: Wired the helper into subscribe_inner prior to state updates, guaranteeing no orphaned or corrupt Subscription storage rows are written if validation fails.

4. Issue #819 — Typed ContractError Conversion for Core Modules

  • Panic Elimination: Removed all raw panic! and .expect() calls across admin.rs, fee.rs, grace.rs, upgrade.rs, and their wrapper sites in lib.rs and storage.rs.
  • Error Register: Added NoPendingAdmin = 42 to errors.rs.
  • Converted Sites Inventory:
Target File Function Call Site Legacy Error Mechanism Converted ContractError
admin.rs accept_admin .expect("no pending admin") ContractError::NoPendingAdmin (42)
storage.rs get_admin .expect("admin not set") ContractError::NotInitialized (7)
grace.rs propose_grace_period assert!(…, "grace period too large") ContractError::AmountExceedsMaximum (15)
lib.rs set_min_interval assert!(…, "min interval must be positive") ContractError::IntervalMustBePositive (3)
lib.rs set_initial_admin panic!("admin already set") ContractError::AlreadyInitialized (1)

🧪 Testing & Validation

All acceptance criteria have been verified against local cargo test suites.

Summary of Added/Updated Unit Tests

Execution Commands

# Run issue-specific test sub-suites
cargo test pause_until
cargo test merchant
cargo test token
cargo test admin fee grace upgrade

# Verify no remaining string panics on modified modules
rg 'panic!|expect\(' contract/src/admin.rs contract/src/fee.rs contract/src/grace.rs contract/src/upgrade.rs

…oken validation, typed errors

SiLioLabs#816 — get_pause_expiry read API + auto-resume event consistency
- Add pub fn get_pause_expiry(env, user) -> Option<u64> in lib.rs
  (delegates to storage::get_pause_expiry, no auth required)
- Verified try_auto_resume in charge_exec.rs already clears PauseExpiry
  and emits subscription_auto_resumed on both charge() and batch_charge() paths
- Tests: get_pause_expiry returns None/expiry/u64::MAX, cleared on resume,
  cleared after charge/batch_charge auto-resume, event assertions

SiLioLabs#817 — enforce MAX_MERCHANT_REVENUE_DAY_INDEX_SIZE with typed overflow error
- Add MAX_MERCHANT_REVENUE_DAY_INDEX_SIZE = 365 constant in merchant_stats.rs
  (documented: ~1 year cap, operators use prune_merchant_revenue_days to free)
- Enforce cap at day-index append time in increment_revenue_with_daily;
  fails closed with ContractError::MerchantDayIndexFull = 41
- Existing-day updates (is_new_day == false) always succeed
- Add MerchantDayIndexFull = 41 to errors.rs
- Tests: cap-1 succeeds, cap+1 fails with SiLioLabs#41, same-day update at cap succeeds

SiLioLabs#818 — validate token contract address + SAC interface on subscribe
- Extract require_valid_token_address() helper in validation.rs
  (XDR byte-7 discriminant check + token::Client::new.decimals() probe)
- Replace inline XDR check in subscribe_inner with the new helper
- Validation fires before any subscription state is written; no row on failure
- Tests: valid SAC succeeds, second token succeeds, non-SAC rejected,
  no subscription row written on failure

SiLioLabs#819 — convert string panic/expect in admin, fee, grace, upgrade to ContractError
- admin.rs accept_admin: .expect('no pending admin') → NoPendingAdmin = 42
- storage.rs get_admin: .expect('admin not set') → NotInitialized = 7
- grace.rs propose_grace_period: assert!(...) → AmountExceedsMaximum = 15
- lib.rs set_min_interval: assert!(...) → IntervalMustBePositive = 3
- lib.rs set_initial_admin: panic!('admin already set') → AlreadyInitialized = 1
- Add NoPendingAdmin = 42 to errors.rs
- Update affected #[should_panic(expected = ...)] tests to use typed error codes
- Delete 8 stale snapshots (auto-regenerated by cargo test with new errors)
- Converted sites: admin.rs/accept_admin, storage.rs/get_admin,
  grace.rs/propose_grace_period, lib.rs/set_min_interval + set_initial_admin
@drips-wave

drips-wave Bot commented Aug 27, 2026

Copy link
Copy Markdown

@talktosam2003 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

talktosam2003 and others added 6 commits August 27, 2026 17:26
…ontractError

- Remove UTF-8 BOM from contract/src/test.rs (introduced as a merge
  artefact, caused the file to fail compilation on Linux CI)

- Restore 8 test snapshots deleted by the PR branch, then update their
  diagnostic events to match the new env.panic_with_error() call sites:
    * 'admin not set' raw panic  -> ContractError::NotInitialized (SiLioLabs#7)
    * 'no pending admin' raw panic -> ContractError::NoPendingAdmin (SiLioLabs#42)
    * 'min interval must be positive' raw panic -> ContractError::IntervalMustBePositive (SiLioLabs#3)

  Each snapshot had its 'log' event block (caught panic string) removed
  and every 'wasm_vm: invalid_action' error replaced with 'contract: N'
  to match what the host records when panic_with_error is used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment