From daa40683e9dbd06016e4adeeee925f79d248843a Mon Sep 17 00:00:00 2001 From: wiesche Date: Wed, 2 Sep 2026 18:33:33 +0200 Subject: [PATCH] validate contract states --- .../tests/contract_self_spend_custom.rs | 13 ++-- controller/tests/contract_srs.rs | 61 ++++++++++++++++++- libwallet/src/contract/actions/README.md | 2 - libwallet/src/contract/actions/sign.rs | 2 + libwallet/src/contract/slate.rs | 21 ++++--- 5 files changed, 82 insertions(+), 17 deletions(-) diff --git a/controller/tests/contract_self_spend_custom.rs b/controller/tests/contract_self_spend_custom.rs index fe24503..92b0aa6 100644 --- a/controller/tests/contract_self_spend_custom.rs +++ b/controller/tests/contract_self_spend_custom.rs @@ -200,10 +200,7 @@ fn wallet_contract_self_spend_custom_tx() -> Result<(), libwallet::Error> { Ok(()) } -#[test] -fn contract_min_confirmations() -> Result<(), libwallet::Error> { - let test_dir = "test_output/contract_min_confirmations"; - setup(test_dir); +fn contract_min_confirmations_impl(test_dir: &'static str) -> Result<(), libwallet::Error> { let (wallets, _, stopper, bh) = create_wallets( vec![ vec![("default", 7)], @@ -295,6 +292,14 @@ fn contract_min_confirmations() -> Result<(), libwallet::Error> { stopper.store(false, Ordering::Relaxed); thread::sleep(Duration::from_millis(200)); + Ok(()) +} + +#[test] +fn contract_min_confirmations() -> Result<(), libwallet::Error> { + let test_dir = "test_output/contract_min_confirmations"; + setup(test_dir); + contract_min_confirmations_impl(test_dir)?; clean_output_dir(test_dir); Ok(()) } diff --git a/controller/tests/contract_srs.rs b/controller/tests/contract_srs.rs index d811437..ea4ef48 100644 --- a/controller/tests/contract_srs.rs +++ b/controller/tests/contract_srs.rs @@ -70,6 +70,41 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> recv_mask, PathBuf::from(test_dir), |api, m| { + let child_index = { + let mut wallet = api.wallet_inst.lock(); + let wallet = wallet.lc_provider()?.wallet_inst()?; + let parent = wallet.parent_key_id(); + wallet.current_child_index(&parent)? + }; + let (_, txs) = api.retrieve_txs(m, false, None, None, None)?; + let mut invalid = slate.clone(); + invalid.state = SlateState::Unknown; + let err = api + .contract_sign( + m, + &invalid, + &ContractSetupArgsAPI { + selection_args: common::contract_selection_args(), + net_change: Some(5_000_000_000), + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!( + err, + libwallet::Error::GenericError(ref msg) + if msg == "Cannot advance a contract slate in state UN" + )); + let current_child_index = { + let mut wallet = api.wallet_inst.lock(); + let wallet = wallet.lc_provider()?.wallet_inst()?; + let parent = wallet.parent_key_id(); + wallet.current_child_index(&parent)? + }; + let (_, current_txs) = api.retrieve_txs(m, false, None, None, None)?; + assert_eq!(current_child_index, child_index); + assert_eq!(current_txs.len(), txs.len()); + let wrong_args = ContractSetupArgsAPI { selection_args: common::contract_selection_args(), net_change: Some(-5_000_000_000), @@ -88,7 +123,31 @@ fn contract_srs_tx_impl(test_dir: &'static str) -> Result<(), libwallet::Error> net_change: Some(5_000_000_000), ..Default::default() }; - slate = api.contract_sign(m, &slate, args)?; + let incoming = slate.clone(); + slate = api.contract_sign(m, &incoming, args)?; + + let child_index = { + let mut wallet = api.wallet_inst.lock(); + let wallet = wallet.lc_provider()?.wallet_inst()?; + let parent = wallet.parent_key_id(); + wallet.current_child_index(&parent)? + }; + let (_, txs) = api.retrieve_txs(m, false, None, None, None)?; + let err = api.contract_sign(m, &incoming, args).unwrap_err(); + assert!(matches!( + err, + libwallet::Error::GenericError(ref msg) + if msg == &format!("Slate with id:{} has already been signed.", incoming.id) + )); + let current_child_index = { + let mut wallet = api.wallet_inst.lock(); + let wallet = wallet.lc_provider()?.wallet_inst()?; + let parent = wallet.parent_key_id(); + wallet.current_child_index(&parent)? + }; + let (_, current_txs) = api.retrieve_txs(m, false, None, None, None)?; + assert_eq!(current_child_index, child_index); + assert_eq!(current_txs.len(), txs.len()); Ok(()) }, )?; diff --git a/libwallet/src/contract/actions/README.md b/libwallet/src/contract/actions/README.md index 503dd71..b14d775 100644 --- a/libwallet/src/contract/actions/README.md +++ b/libwallet/src/contract/actions/README.md @@ -73,8 +73,6 @@ Separating side effects until the 'save_step' part would make these functions mu #### Tests - - signing the same slate twice, which `verify_not_signed` guards - - invalid or inconsistent slates and unexpected states - slate contents at each step, not only the end state - the foreign API entry points for `new` and `sign` - Slatepack version negotiation with the last supported wallet release diff --git a/libwallet/src/contract/actions/sign.rs b/libwallet/src/contract/actions/sign.rs index e0b8ecd..9e6cb8b 100644 --- a/libwallet/src/contract/actions/sign.rs +++ b/libwallet/src/contract/actions/sign.rs @@ -98,6 +98,8 @@ where { let mut sl = slate.clone(); contract::utils::verify_not_signed(w, sl.id)?; + // Reject terminal and unknown states before setup reserves keys. + contract::slate::next_state(&sl.state)?; // Ensure net_change has been provided let expected_net_change = diff --git a/libwallet/src/contract/slate.rs b/libwallet/src/contract/slate.rs index 34e3264..4c1e7d8 100644 --- a/libwallet/src/contract/slate.rs +++ b/libwallet/src/contract/slate.rs @@ -438,26 +438,27 @@ where Ok(()) } -/// Transition the slate state to the next one -pub fn transition_state(slate: &mut Slate) -> Result<(), Error> { - // We don't really use these states right now apart from leaving it to derive expected net_change. - // This suggests these can't be used for manipulation. It doesn't hurt to think a bit more if that's the case. - let new_state = match slate.state { +/// Return the next contract slate state +pub(super) fn next_state(state: &SlateState) -> Result { + let next = match state { SlateState::Invoice1 => SlateState::Invoice2, SlateState::Invoice2 => SlateState::Invoice3, SlateState::Standard1 => SlateState::Standard2, SlateState::Standard2 => SlateState::Standard3, - // Unknown, or a slate that has already reached its final state, is not something - // we can advance. We have signed by this point and the wallet state is persisted - // next, so report it rather than reporting a Standard3 that never happened. - ref s => { + // Unknown and final states cannot be advanced. + s => { return Err(Error::GenericError(format!( "Cannot advance a contract slate in state {}", s ))) } }; - slate.state = new_state; + Ok(next) +} + +/// Transition the slate state to the next one +pub(super) fn transition_state(slate: &mut Slate) -> Result<(), Error> { + slate.state = next_state(&slate.state)?; // NOTE: It's possible to never reach the step3. A self-spend has only 2 steps: new -> sign. Ok(()) }