mirror of
https://github.com/jwinterm/grin-wallet.git
synced 2026-10-17 07:11:36 +02:00
Contracts: make revoke idempotent and resumable
revoke cancels the original tx (unlocking its inputs) before the replacement self-spend is built, and the steps commit separately, so a crash in between left the inputs Unspent with no self-spend while a retry silently did nothing. - Find the contributed inputs by tx id whether they are still Locked (original tx active) or already Unspent (a prior revoke cancelled them but the self-spend did not finish), instead of Locked-only. - Cancel only when the tx is still in a cancellable state, so a resumed or repeated revoke skips the cancel and re-spends the Unspent inputs. - Once the self-spend completes the inputs reference its tx id, so a later revoke is a no-op. Adds a regression test that interrupts a revoke after the cancel and confirms the next revoke resumes and produces the self-spend. Addresses PR #754 review (revoke.rs).
This commit is contained in:
jwinterm
@@ -121,3 +121,75 @@ fn wallet_contract_revoke_other_account() -> Result<(), libwallet::Error> {
|
||||
clean_output_dir(test_dir);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A revoke interrupted right after the cancel (inputs already unlocked, no self-spend
|
||||
/// built yet) must be resumable: a second revoke detects the now-Unspent inputs that are
|
||||
/// still tagged with the cancelled tx and still produces the self-spend, rather than
|
||||
/// silently doing nothing.
|
||||
fn contract_revoke_resume_impl(test_dir: &'static str) -> Result<(), libwallet::Error> {
|
||||
let (wallets, _chain, stopper, _bh) =
|
||||
create_wallets(vec![vec![("default", 4)]], test_dir).unwrap();
|
||||
let wallet1 = wallets[0].0.clone();
|
||||
let mask1 = wallets[0].1.as_ref();
|
||||
|
||||
// Send (with early lock), locking an input under the contract tx.
|
||||
let mut slate = Slate::blank(0, true);
|
||||
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| {
|
||||
let args = &ContractNewArgsAPI {
|
||||
setup_args: ContractSetupArgsAPI {
|
||||
net_change: Some(-1_000_000_000),
|
||||
num_participants: 2,
|
||||
add_outputs: true,
|
||||
..Default::default()
|
||||
},
|
||||
..Default::default()
|
||||
};
|
||||
slate = api.contract_new(m, args)?;
|
||||
Ok(())
|
||||
})?;
|
||||
assert_eq!(slate.state, SlateState::Standard1);
|
||||
|
||||
let mut tx_id = 0;
|
||||
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| {
|
||||
let (_, txs) = api.retrieve_txs(m, true, None, None, None)?;
|
||||
tx_id = txs.last().unwrap().id;
|
||||
Ok(())
|
||||
})?;
|
||||
|
||||
// Simulate a revoke that crashed right after cancelling: cancel the tx (inputs become
|
||||
// Unspent, still tagged with tx_id) but do NOT build the self-spend.
|
||||
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| {
|
||||
api.cancel_tx(m, Some(tx_id), None)?;
|
||||
let (_, outs) = api.retrieve_outputs(m, true, false, None)?;
|
||||
assert!(
|
||||
outs.iter().any(|o| o.output.status == OutputStatus::Unspent
|
||||
&& o.output.tx_log_entry == Some(tx_id)),
|
||||
"input should be Unspent but still tagged with the cancelled tx"
|
||||
);
|
||||
Ok(())
|
||||
})?;
|
||||
|
||||
// Resume: revoke must still produce the self-spend from the now-Unspent input.
|
||||
let mut revoked = None;
|
||||
wallet::controller::owner_single_use(Some(wallet1.clone()), mask1, None, |api, m| {
|
||||
revoked = api.contract_revoke(m, &ContractRevokeArgsAPI { tx_id })?;
|
||||
Ok(())
|
||||
})?;
|
||||
assert!(
|
||||
revoked.is_some(),
|
||||
"interrupted revoke should resume and produce a self-spend slate"
|
||||
);
|
||||
|
||||
stopper.store(false, Ordering::Relaxed);
|
||||
thread::sleep(Duration::from_millis(200));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn wallet_contract_revoke_resume() -> Result<(), libwallet::Error> {
|
||||
let test_dir = "test_output/contract_revoke_resume";
|
||||
setup(test_dir);
|
||||
contract_revoke_resume_impl(test_dir)?;
|
||||
clean_output_dir(test_dir);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -20,7 +20,7 @@ use crate::grin_keychain::Keychain;
|
||||
use crate::grin_util::secp::key::SecretKey;
|
||||
use crate::internal::tx;
|
||||
use crate::slate::Slate;
|
||||
use crate::types::{NodeClient, OutputData, OutputStatus};
|
||||
use crate::types::{NodeClient, OutputData, OutputStatus, TxLogEntryType};
|
||||
use crate::backend::WalletBackend;
|
||||
|
||||
/// Contract revocation is done by double-spending the input
|
||||
@@ -33,55 +33,70 @@ where
|
||||
C: NodeClient,
|
||||
K: Keychain,
|
||||
{
|
||||
// TODO: check the correctness of this. This is essentially old cancel + self-spend.
|
||||
// FUTURE: we may want to boost fees in case we notice something in the mempool. There
|
||||
// are also race conditions possible. We may not want to label txlogenry as Canceled
|
||||
// until the new tx gets on the chain.
|
||||
|
||||
// If we contributed inputs, we must have locked them at which point we also set the
|
||||
// OutputData.tx_log_entry which is the tx_id.
|
||||
// Revoke double-spends an input we contributed to tx_id. cancel_tx, the self-spend
|
||||
// new() and sign() each commit separately, so these steps are NOT one atomic unit.
|
||||
// revoke() is therefore written to be safely re-invocable: a crash between cancelling
|
||||
// and finishing the self-spend leaves our inputs Unspent but still tagged with tx_id,
|
||||
// and a second call resumes from there. cancel_tx must run first because a Locked
|
||||
// output is ineligible for selection (OutputData::eligible_to_spend), so the self-spend
|
||||
// cannot be built until the input is unlocked.
|
||||
// FUTURE: we may want to boost fees if we notice the original tx in the mempool.
|
||||
let tx_id = args.tx_id;
|
||||
|
||||
// Find my outputs that have been Locked and refer to the given tx_id
|
||||
// Inputs we contributed to tx_id that are still recoverable. Locked => the original tx
|
||||
// is still active; Unspent => a previous revoke cancelled it but the self-spend did not
|
||||
// finish. Once the self-spend completes these reference the self-spend's tx id instead
|
||||
// and are no longer matched here, which makes a repeat revoke a no-op.
|
||||
let my_contributed_inputs = w
|
||||
.batch(keychain_mask)?
|
||||
.iter()?
|
||||
.filter(|out| {
|
||||
// Find an output that is Locked and is in the tx_input_commit
|
||||
out.status == OutputStatus::Locked
|
||||
&& (out.tx_log_entry.is_some() && out.tx_log_entry.as_ref().unwrap() == &tx_id)
|
||||
out.tx_log_entry == Some(tx_id)
|
||||
&& (out.status == OutputStatus::Locked || out.status == OutputStatus::Unspent)
|
||||
})
|
||||
.collect::<Vec<OutputData>>();
|
||||
|
||||
// Determine the account that owns the locked inputs so the cancel and the self-spend
|
||||
// target it rather than whatever account happens to be active.
|
||||
// Determine the account that owns the inputs so the cancel and the self-spend target it
|
||||
// rather than whatever account happens to be active.
|
||||
let parent_key_id = match my_contributed_inputs.first() {
|
||||
Some(out) => out.root_key_id.clone(),
|
||||
None => w.parent_key_id(),
|
||||
};
|
||||
|
||||
// The revoked transaction's slate id, so we can drop its private context below.
|
||||
let revoked_slate_id = w
|
||||
.get_tx_log_entry(parent_key_id.clone(), tx_id)?
|
||||
.and_then(|e| e.tx_slate_id);
|
||||
|
||||
// 1. Unlock the input by calling cancel_tx
|
||||
tx::cancel_tx(&mut *w, keychain_mask, &parent_key_id, Some(tx_id), None)?;
|
||||
|
||||
// Drop the canceled slate's private context if one still exists (signing a contract
|
||||
// already deletes it).
|
||||
if let Some(slate_id) = revoked_slate_id {
|
||||
if w
|
||||
.get_private_context(keychain_mask, slate_id.as_bytes())
|
||||
.is_ok()
|
||||
{
|
||||
let mut batch = w.batch(keychain_mask)?;
|
||||
batch.delete_private_context(slate_id.as_bytes())?;
|
||||
batch.commit()?;
|
||||
// Cancel the original tx only if it is still in a cancellable state. On a resumed revoke
|
||||
// it is already a *Cancelled type (and the inputs are Unspent), so we skip straight to
|
||||
// re-spending them.
|
||||
let revoked = w.get_tx_log_entry(parent_key_id.clone(), tx_id)?;
|
||||
let needs_cancel = match revoked.as_ref() {
|
||||
Some(e) => matches!(
|
||||
e.tx_type,
|
||||
TxLogEntryType::TxSent
|
||||
| TxLogEntryType::TxReceived
|
||||
| TxLogEntryType::TxReverted
|
||||
| TxLogEntryType::TxSelfSpend
|
||||
),
|
||||
None => false,
|
||||
};
|
||||
if needs_cancel {
|
||||
// 1. Unlock the inputs by cancelling the original tx.
|
||||
tx::cancel_tx(&mut *w, keychain_mask, &parent_key_id, Some(tx_id), None)?;
|
||||
// Drop the canceled slate's private context if one still exists (signing already
|
||||
// deletes it).
|
||||
if let Some(slate_id) = revoked.and_then(|e| e.tx_slate_id) {
|
||||
if w
|
||||
.get_private_context(keychain_mask, slate_id.as_bytes())
|
||||
.is_ok()
|
||||
{
|
||||
let mut batch = w.batch(keychain_mask)?;
|
||||
batch.delete_private_context(slate_id.as_bytes())?;
|
||||
batch.commit()?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if my_contributed_inputs.len() == 0 {
|
||||
// Nothing of ours to double-spend: we contributed no inputs, or a prior revoke already
|
||||
// re-spent them.
|
||||
if my_contributed_inputs.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
let input_commit = my_contributed_inputs[0].commit.as_ref().ok_or_else(|| {
|
||||
|
||||
Reference in New Issue
Block a user