Skip to content

Commit b3bb391

Browse files
mbjorkqvistclaude
andcommitted
fix(cketh): report a failed funding as failed, not pending reimbursement
`processed_transaction_status` classified every failed finalized transaction as `PendingReimbursement`, which the Candid interface defines as "transaction failed and will be reimbursed". A sweeper funding is never reimbursed, so `retrieve_eth_status` and `withdrawal_status` promised a reimbursement that nothing will ever settle — and the status would stay wrong forever, since only recording a reimbursement moves it on. Add a `Failed` variant to `TxFinalizedStatus` for a failure that will not be reimbursed, and pick between the two by asking the request whether it is reimbursable. This needs the didc override, as any addition to a returned variant does. Tests cover all three paths: a failed funding reports `Failed`, a successful one still reports `Success`, and a failed *user* withdrawal still reports `PendingReimbursement` — without that last one the first would also pass if the branch were inverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7336271 commit b3bb391

4 files changed

Lines changed: 71 additions & 8 deletions

File tree

‎rs/ethereum/cketh/minter/cketh_minter.did‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,8 @@ type TxFinalizedStatus = variant {
273273
};
274274
// Transaction failed and will be reimbursed,
275275
PendingReimbursement : EthTransaction;
276+
// Transaction failed and will not be reimbursed.
277+
Failed : EthTransaction;
276278
};
277279

278280
// Retrieve the status of a withdrawal request.

‎rs/ethereum/cketh/minter/src/endpoints.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ pub enum TxFinalizedStatus {
171171
effective_transaction_fee: Option<Nat>,
172172
},
173173
PendingReimbursement(EthTransaction),
174+
Failed(EthTransaction),
174175
Reimbursed {
175176
transaction_hash: String,
176177
reimbursed_amount: Nat,
@@ -192,6 +193,9 @@ impl Display for RetrieveEthStatus {
192193
TxFinalizedStatus::PendingReimbursement(tx) => {
193194
write!(f, "PendingReimbursement({})", tx.transaction_hash)
194195
}
196+
TxFinalizedStatus::Failed(tx) => {
197+
write!(f, "Failed({})", tx.transaction_hash)
198+
}
195199
TxFinalizedStatus::Reimbursed {
196200
reimbursed_in_block,
197201
transaction_hash,

‎rs/ethereum/cketh/minter/src/state/transactions/mod.rs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,14 +1000,18 @@ impl EthTransactions {
10001000
);
10011001
}
10021002
if tx.transaction_status() == &TransactionStatus::Failure {
1003-
return (
1004-
RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement(
1005-
EthTransaction {
1006-
transaction_hash: tx.transaction_hash().to_string(),
1007-
},
1008-
)),
1009-
Some(tx.as_ref()),
1010-
);
1003+
let failure = EthTransaction {
1004+
transaction_hash: tx.transaction_hash().to_string(),
1005+
};
1006+
// A request that cannot be reimbursed must not report a pending reimbursement:
1007+
// nothing will ever settle it, so the status would stay wrong forever.
1008+
let status = match self.processed_withdrawal_requests.get(burn_index) {
1009+
Some(request) if !request.is_reimbursable() => {
1010+
TxFinalizedStatus::Failed(failure)
1011+
}
1012+
_ => TxFinalizedStatus::PendingReimbursement(failure),
1013+
};
1014+
return (RetrieveEthStatus::TxFinalized(status), Some(tx.as_ref()));
10111015
}
10121016

10131017
return (

‎rs/ethereum/cketh/minter/src/state/transactions/tests.rs‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2126,6 +2126,7 @@ mod eth_transactions {
21262126
mod sweeper_funding {
21272127
use super::withdrawal_flow;
21282128
use super::*;
2129+
use crate::endpoints::{RetrieveEthStatus, TxFinalizedStatus};
21292130
use crate::eth_logs::LedgerSubaccount;
21302131
use crate::lifecycle::EthereumNetwork;
21312132
use crate::numeric::TransactionCount;
@@ -2208,6 +2209,58 @@ mod eth_transactions {
22082209
}
22092210
}
22102211

2212+
#[test]
2213+
fn should_report_a_failed_funding_as_failed_not_pending_reimbursement() {
2214+
let mut transactions = EthTransactions::new(TransactionNonce::ZERO);
2215+
let funding = sweeper_funding_request();
2216+
let burn_index = funding.ledger_burn_index;
2217+
2218+
let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Failure);
2219+
2220+
assert_eq!(
2221+
transactions.transaction_status(&burn_index),
2222+
RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Failed((&receipt).into())),
2223+
"nothing will ever reimburse a funding, so PendingReimbursement would stay wrong \
2224+
forever"
2225+
);
2226+
}
2227+
2228+
#[test]
2229+
fn should_report_a_successful_funding_as_success() {
2230+
let mut transactions = EthTransactions::new(TransactionNonce::ZERO);
2231+
let funding = sweeper_funding_request();
2232+
let burn_index = funding.ledger_burn_index;
2233+
2234+
let receipt = withdrawal_flow(&mut transactions, funding, TransactionStatus::Success);
2235+
2236+
assert_matches!(
2237+
transactions.transaction_status(&burn_index),
2238+
RetrieveEthStatus::TxFinalized(TxFinalizedStatus::Success {
2239+
transaction_hash,
2240+
..
2241+
}) if transaction_hash == receipt.transaction_hash.to_string()
2242+
);
2243+
}
2244+
2245+
#[test]
2246+
fn should_still_report_a_failed_user_withdrawal_as_pending_reimbursement() {
2247+
let mut transactions = EthTransactions::new(TransactionNonce::ZERO);
2248+
let burn_index = LedgerBurnIndex::new(15);
2249+
2250+
let receipt = withdrawal_flow(
2251+
&mut transactions,
2252+
cketh_withdrawal_request_with_index(burn_index),
2253+
TransactionStatus::Failure,
2254+
);
2255+
2256+
assert_eq!(
2257+
transactions.transaction_status(&burn_index),
2258+
RetrieveEthStatus::TxFinalized(TxFinalizedStatus::PendingReimbursement(
2259+
(&receipt).into()
2260+
))
2261+
);
2262+
}
2263+
22112264
#[test]
22122265
fn should_still_reimburse_a_failed_user_withdrawal() {
22132266
let mut transactions = EthTransactions::new(TransactionNonce::ZERO);

0 commit comments

Comments
 (0)