From 1e6db6c0c390fff9f139183d131df10fc7c7c39c Mon Sep 17 00:00:00 2001 From: Heorhii Azarov Date: Fri, 10 May 2024 15:41:51 +0300 Subject: [PATCH] Add tests and minor fixes --- .../src/constraints_accumulator.rs | 4 +- .../src/tests/orders_constraints.rs | 6 +- chainstate/src/detail/ban_score.rs | 1 + chainstate/src/detail/error_classification.rs | 1 + .../test-suite/src/tests/orders_tests.rs | 138 +++++++++++++++++- mempool/src/error/ban_score.rs | 1 + orders-accounting/src/cache.rs | 4 +- orders-accounting/src/error.rs | 16 +- orders-accounting/src/price_calculation.rs | 17 ++- orders-accounting/src/tests/operations.rs | 24 +-- 10 files changed, 184 insertions(+), 28 deletions(-) diff --git a/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs b/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs index 30badfb929..0dbe08fb00 100644 --- a/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs +++ b/chainstate/constraints-value-accumulator/src/constraints_accumulator.rs @@ -279,12 +279,12 @@ impl ConstrainedValueAccumulator { .ok_or(orders_accounting::Error::OrderGiveBalanceNotFound(*id))?; let initially_asked = output_value_amount(order_data.ask())?; - let ask_amount = (initially_asked - ask_balance) + let filled_amount = (initially_asked - ask_balance) .ok_or(Error::NegativeAccountBalance(AccountType::Order(*id)))?; let ask_currency = CoinOrTokenId::from_output_value(order_data.ask()) .ok_or(Error::UnsupportedTokenVersion)?; - insert_or_increase(&mut self.unconstrained_value, ask_currency, ask_amount)?; + insert_or_increase(&mut self.unconstrained_value, ask_currency, filled_amount)?; let give_currency = CoinOrTokenId::from_output_value(order_data.give()) .ok_or(Error::UnsupportedTokenVersion)?; diff --git a/chainstate/constraints-value-accumulator/src/tests/orders_constraints.rs b/chainstate/constraints-value-accumulator/src/tests/orders_constraints.rs index a2b878f744..802d7c3c98 100644 --- a/chainstate/constraints-value-accumulator/src/tests/orders_constraints.rs +++ b/chainstate/constraints-value-accumulator/src/tests/orders_constraints.rs @@ -307,7 +307,11 @@ fn fill_order_constraints(#[case] seed: Seed) { assert_eq!( result.unwrap_err(), - Error::OrdersAccountingError(orders_accounting::Error::OrderOverflow(order_id)) + Error::OrdersAccountingError(orders_accounting::Error::OrderOverbid( + order_id, + ask_amount, + (ask_amount + Amount::from_atoms(1)).unwrap() + )) ); } diff --git a/chainstate/src/detail/ban_score.rs b/chainstate/src/detail/ban_score.rs index 857c28df13..8ab26695b0 100644 --- a/chainstate/src/detail/ban_score.rs +++ b/chainstate/src/detail/ban_score.rs @@ -625,6 +625,7 @@ impl BanScore for orders_accounting::Error { Error::FillOrderChangeLeft(_) => 100, Error::CurrencyMismatch => 100, Error::OrderOverflow(_) => 100, + Error::OrderOverbid(_, _, _) => 100, Error::AttemptedCancelNonexistingOrderData(_) => 100, Error::AttemptedCancelNonexistingAskBalance(_) => 100, Error::AttemptedCancelNonexistingGiveBalance(_) => 100, diff --git a/chainstate/src/detail/error_classification.rs b/chainstate/src/detail/error_classification.rs index f4624762f3..66e1d9d501 100644 --- a/chainstate/src/detail/error_classification.rs +++ b/chainstate/src/detail/error_classification.rs @@ -835,6 +835,7 @@ impl BlockProcessingErrorClassification for orders_accounting::Error { | Error::FillOrderChangeLeft(_) | Error::CurrencyMismatch | Error::OrderOverflow(_) + | Error::OrderOverbid(_, _, _) | Error::AttemptedCancelNonexistingOrderData(_) | Error::AttemptedCancelNonexistingAskBalance(_) | Error::AttemptedCancelNonexistingGiveBalance(_) diff --git a/chainstate/test-suite/src/tests/orders_tests.rs b/chainstate/test-suite/src/tests/orders_tests.rs index d2a2d490b2..94e2b6fd7d 100644 --- a/chainstate/test-suite/src/tests/orders_tests.rs +++ b/chainstate/test-suite/src/tests/orders_tests.rs @@ -16,6 +16,7 @@ use chainstate_storage::Transactional; use chainstate_test_framework::{TestFramework, TransactionBuilder}; use common::{ + address::pubkeyhash::PublicKeyHash, chain::{ make_order_id, output_value::OutputValue, @@ -132,7 +133,7 @@ fn create_order_check_storage(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn create_two_2_orders_same_tx(#[case] seed: Seed) { +fn create_two_same_orders_in_tx(#[case] seed: Seed) { utils::concurrency::model(move || { let mut rng = make_seedable_rng(seed); let mut tf = TestFramework::builder(&mut rng).build(); @@ -171,7 +172,52 @@ fn create_two_2_orders_same_tx(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn create_two_2_orders_same_block(#[case] seed: Seed) { +fn create_two_orders_same_tx(#[case] seed: Seed) { + utils::concurrency::model(move || { + let mut rng = make_seedable_rng(seed); + let mut tf = TestFramework::builder(&mut rng).build(); + + let (token_id, tokens_outpoint, _) = issue_and_mint_token_from_genesis(&mut rng, &mut tf); + + let amount1 = Amount::from_atoms(rng.gen_range(1u128..1000)); + let amount2 = Amount::from_atoms(rng.gen_range(1u128..1000)); + let order_data_1 = OrderData::new( + Destination::AnyoneCanSpend, + OutputValue::Coin(amount1), + OutputValue::TokenV1(token_id, amount2), + ); + + let order_data_2 = OrderData::new( + Destination::PublicKeyHash(PublicKeyHash::random()), + OutputValue::Coin(amount2), + OutputValue::TokenV1(token_id, amount1), + ); + + let order_id = make_order_id(&tokens_outpoint); + let tx = TransactionBuilder::new() + .add_input(tokens_outpoint.into(), InputWitness::NoSignature(None)) + .add_output(TxOutput::AnyoneCanTake(order_data_1)) + .add_output(TxOutput::AnyoneCanTake(order_data_2)) + .build(); + let result = tf.make_block_builder().add_transaction(tx).build_and_process(&mut rng); + + assert_eq!( + result.unwrap_err(), + chainstate::ChainstateError::ProcessBlockError( + chainstate::BlockError::StateUpdateFailed( + chainstate::ConnectTransactionError::OrdersAccountingError( + orders_accounting::Error::OrderAlreadyExists(order_id) + ) + ) + ) + ); + }); +} + +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn create_two_orders_same_block(#[case] seed: Seed) { utils::concurrency::model(move || { let mut rng = make_seedable_rng(seed); let mut tf = TestFramework::builder(&mut rng).build(); @@ -509,6 +555,94 @@ fn fill_order_check_storage(#[case] seed: Seed) { }); } +#[rstest] +#[trace] +#[case(Seed::from_entropy())] +fn fill_partially_then_cancel(#[case] seed: Seed) { + utils::concurrency::model(move || { + let mut rng = make_seedable_rng(seed); + let mut tf = TestFramework::builder(&mut rng).build(); + + let (token_id, tokens_outpoint, coins_outpoint) = + issue_and_mint_token_from_genesis(&mut rng, &mut tf); + + let ask_amount = Amount::from_atoms(rng.gen_range(1u128..1000)); + let give_amount = Amount::from_atoms(rng.gen_range(1u128..1000)); + let order_data = OrderData::new( + Destination::AnyoneCanSpend, + OutputValue::Coin(ask_amount), + OutputValue::TokenV1(token_id, give_amount), + ); + + let order_id = make_order_id(&tokens_outpoint); + let tx = TransactionBuilder::new() + .add_input(tokens_outpoint.into(), InputWitness::NoSignature(None)) + .add_output(TxOutput::AnyoneCanTake(order_data.clone())) + .build(); + tf.make_block_builder().add_transaction(tx).build_and_process(&mut rng).unwrap(); + + // Fill the order partially + let fill_value = OutputValue::Coin(Amount::from_atoms( + rng.gen_range(1..ask_amount.into_atoms()), + )); + let filled_amount = { + let db_tx = tf.storage.transaction_ro().unwrap(); + let orders_db = OrdersAccountingDB::new(&db_tx); + orders_accounting::calculate_fill_order(&orders_db, order_id, &fill_value).unwrap() + }; + + let tx = TransactionBuilder::new() + .add_input(coins_outpoint.into(), InputWitness::NoSignature(None)) + .add_input( + TxInput::AccountCommand( + AccountNonce::new(0), + AccountCommand::FillOrder( + order_id, + fill_value.clone(), + Destination::AnyoneCanSpend, + ), + ), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::TokenV1(token_id, filled_amount), + Destination::AnyoneCanSpend, + )) + .build(); + tf.make_block_builder().add_transaction(tx).build_and_process(&mut rng).unwrap(); + + // cancel the order + let tx = TransactionBuilder::new() + .add_input( + TxInput::AccountCommand( + AccountNonce::new(1), + AccountCommand::CancelOrder(order_id), + ), + InputWitness::NoSignature(None), + ) + .add_output(TxOutput::Transfer( + OutputValue::TokenV1(token_id, (give_amount - filled_amount).unwrap()), + Destination::AnyoneCanSpend, + )) + .add_output(TxOutput::Transfer( + OutputValue::Coin(output_value_amount(&fill_value)), + Destination::AnyoneCanSpend, + )) + .build(); + tf.make_block_builder().add_transaction(tx).build_and_process(&mut rng).unwrap(); + + assert_eq!(None, tf.chainstate.get_order_data(&order_id).unwrap()); + assert_eq!( + None, + tf.chainstate.get_order_ask_balance(&order_id).unwrap() + ); + assert_eq!( + None, + tf.chainstate.get_order_give_balance(&order_id).unwrap() + ); + }); +} + #[rstest] #[trace] #[case(Seed::from_entropy())] diff --git a/mempool/src/error/ban_score.rs b/mempool/src/error/ban_score.rs index 676181e492..25e770d9ce 100644 --- a/mempool/src/error/ban_score.rs +++ b/mempool/src/error/ban_score.rs @@ -458,6 +458,7 @@ impl MempoolBanScore for orders_accounting::Error { Error::FillOrderChangeLeft(_) => 100, Error::CurrencyMismatch => 100, Error::OrderOverflow(_) => 100, + Error::OrderOverbid(_, _, _) => 100, Error::AttemptedCancelNonexistingOrderData(_) => 0, Error::AttemptedCancelNonexistingAskBalance(_) => 0, Error::AttemptedCancelNonexistingGiveBalance(_) => 0, diff --git a/orders-accounting/src/cache.rs b/orders-accounting/src/cache.rs index 750bcf363a..0b6a903295 100644 --- a/orders-accounting/src/cache.rs +++ b/orders-accounting/src/cache.rs @@ -84,7 +84,7 @@ impl OrdersAccountingCache

{ Ok(()) } - fn undo_withdraw_order(&mut self, undo: CancelOrderUndo) -> Result<()> { + fn undo_cancel_order(&mut self, undo: CancelOrderUndo) -> Result<()> { ensure!( self.get_order_data(&undo.id)?.is_none(), Error::InvariantOrderDataExistForCancelUndo(undo.id) @@ -242,7 +242,7 @@ impl OrdersAccountingOperations for OrdersAccountingCac fn undo(&mut self, undo_data: OrdersAccountingUndo) -> Result<()> { match undo_data { OrdersAccountingUndo::CreateOrder(undo) => self.undo_create_order(undo), - OrdersAccountingUndo::CancelOrder(undo) => self.undo_withdraw_order(undo), + OrdersAccountingUndo::CancelOrder(undo) => self.undo_cancel_order(undo), OrdersAccountingUndo::FillOrder(undo) => self.undo_fill_order(undo), } } diff --git a/orders-accounting/src/error.rs b/orders-accounting/src/error.rs index 6a59b31033..14753ac94b 100644 --- a/orders-accounting/src/error.rs +++ b/orders-accounting/src/error.rs @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use common::chain::OrderId; +use common::{chain::OrderId, primitives::Amount}; #[derive(thiserror::Error, Debug, PartialEq, Eq, Clone)] pub enum Error { @@ -39,11 +39,11 @@ pub enum Error { InvariantOrderGiveBalanceNotFoundForUndo(OrderId), #[error("Give balance for order {0}` changed for undo")] InvariantOrderGiveBalanceChangedForUndo(OrderId), - #[error("Data for order {0}` still exist on withdraw undo")] + #[error("Data for order {0}` still exist on cancel undo")] InvariantOrderDataExistForCancelUndo(OrderId), - #[error("Ask balance for order {0}` still exist on withdraw undo")] + #[error("Ask balance for order {0}` still exist on cancel undo")] InvariantOrderAskBalanceExistForCancelUndo(OrderId), - #[error("Give balance for order {0}` still exist on withdraw undo")] + #[error("Give balance for order {0}` still exist on cancel undo")] InvariantOrderGiveBalanceExistForCancelUndo(OrderId), #[error("Fill operation for order {0}` left a change")] FillOrderChangeLeft(OrderId), @@ -51,11 +51,13 @@ pub enum Error { CurrencyMismatch, #[error("Order overflow: `{0}`")] OrderOverflow(OrderId), - #[error("Attempt to withdraw non-existing order data `{0}`")] + #[error("Order `{0}` can provide `{1:?}` amount; but attempted to fill `{2:?}`")] + OrderOverbid(OrderId, Amount, Amount), + #[error("Attempt to cancel non-existing order data `{0}`")] AttemptedCancelNonexistingOrderData(OrderId), - #[error("Attempt to withdraw non-existing ask balance `{0}`")] + #[error("Attempt to cancel non-existing ask balance `{0}`")] AttemptedCancelNonexistingAskBalance(OrderId), - #[error("Attempt to withdraw non-existing give balance `{0}`")] + #[error("Attempt to cancel non-existing give balance `{0}`")] AttemptedCancelNonexistingGiveBalance(OrderId), #[error("Unsupported token version")] UnsupportedTokenVersion, diff --git a/orders-accounting/src/price_calculation.rs b/orders-accounting/src/price_calculation.rs index d7b16b5f2e..ad005a8a24 100644 --- a/orders-accounting/src/price_calculation.rs +++ b/orders-accounting/src/price_calculation.rs @@ -64,11 +64,17 @@ fn ensure_currencies_and_amounts_match( ) -> Result<()> { match (left, right) { (OutputValue::Coin(amount1), OutputValue::Coin(amount2)) => { - ensure!(amount1 >= amount2, Error::OrderOverflow(order_id)); + ensure!( + amount1 >= amount2, + Error::OrderOverbid(order_id, *amount1, *amount2) + ); Ok(()) } (OutputValue::TokenV1(id1, amount1), OutputValue::TokenV1(id2, amount2)) => { - ensure!(amount1 >= amount2, Error::OrderOverflow(order_id)); + ensure!( + amount1 >= amount2, + Error::OrderOverbid(order_id, *amount1, *amount2) + ); ensure!(id1 == id2, Error::CurrencyMismatch); Ok(()) } @@ -166,9 +172,10 @@ mod tests { #[rstest] #[case(token!(0), coin!(1), token!(0), Error::OrderOverflow(OrderId::zero()))] - #[case(token!(0), coin!(1), token!(1), Error::OrderOverflow(OrderId::zero()))] - #[case(coin!(1), token!(1), coin!(2), Error::OrderOverflow(OrderId::zero()))] - #[case(coin!(1), token!(u128::MAX), coin!(2), Error::OrderOverflow(OrderId::zero()))] + #[case(token!(0), coin!(1), token!(1), Error::OrderOverbid(OrderId::zero(), Amount::from_atoms(0), Amount::from_atoms(1)))] + #[case(coin!(1), token!(1), coin!(2), Error::OrderOverbid(OrderId::zero(), Amount::from_atoms(1), Amount::from_atoms(2)))] + #[case(coin!(1), token!(u128::MAX), coin!(2), Error::OrderOverbid(OrderId::zero(), Amount::from_atoms(1), Amount::from_atoms(2)))] + #[case(coin!(2), token!(u128::MAX), coin!(2), Error::OrderOverflow(OrderId::zero()))] #[case(coin!(1), token!(1), token!(1), Error::CurrencyMismatch)] #[case(coin!(1), token!(1), token!(1), Error::CurrencyMismatch)] #[case(token!(1), token2!(1), token2!(1), Error::CurrencyMismatch)] diff --git a/orders-accounting/src/tests/operations.rs b/orders-accounting/src/tests/operations.rs index 42c553d8ae..3aaa79d52f 100644 --- a/orders-accounting/src/tests/operations.rs +++ b/orders-accounting/src/tests/operations.rs @@ -144,7 +144,7 @@ fn create_order_and_undo(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn withdraw_order_and_flush(#[case] seed: Seed) { +fn cancel_order_and_flush(#[case] seed: Seed) { let mut rng = make_seedable_rng(seed); let order_id = OrderId::random_using(&mut rng); @@ -158,7 +158,7 @@ fn withdraw_order_and_flush(#[case] seed: Seed) { let mut db = OrdersAccountingDB::new(&mut storage); let mut cache = OrdersAccountingCache::new(&db); - // try to withdraw non-existing order + // try to cancel non-existing order { let random_order = OrderId::random_using(&mut rng); let result = cache.cancel_order(random_order); @@ -178,7 +178,7 @@ fn withdraw_order_and_flush(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn withdraw_order_twice(#[case] seed: Seed) { +fn cancel_order_twice(#[case] seed: Seed) { let mut rng = make_seedable_rng(seed); let order_id = OrderId::random_using(&mut rng); @@ -203,7 +203,7 @@ fn withdraw_order_twice(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn withdraw_order_and_undo(#[case] seed: Seed) { +fn cancel_order_and_undo(#[case] seed: Seed) { let mut rng = make_seedable_rng(seed); let order_id = OrderId::random_using(&mut rng); @@ -276,11 +276,17 @@ fn fill_entire_order_and_flush(#[case] seed: Seed) { // try to overfill { - let fill = OutputValue::Coin( - (output_value_amount(order_data.ask()).unwrap() + Amount::from_atoms(1)).unwrap(), - ); + let ask_amount = output_value_amount(order_data.ask()).unwrap(); + let fill = OutputValue::Coin((ask_amount + Amount::from_atoms(1)).unwrap()); let result = cache.fill_order(order_id, fill); - assert_eq!(result.unwrap_err(), Error::OrderOverflow(order_id)); + assert_eq!( + result.unwrap_err(), + Error::OrderOverbid( + order_id, + ask_amount, + (ask_amount + Amount::from_atoms(1)).unwrap() + ) + ); } let _ = cache.fill_order(order_id, order_data.ask().clone()).unwrap(); @@ -471,7 +477,7 @@ fn fill_order_partially_and_undo(#[case] seed: Seed) { #[rstest] #[trace] #[case(Seed::from_entropy())] -fn fill_order_partially_and_withdraw(#[case] seed: Seed) { +fn fill_order_partially_and_cancel(#[case] seed: Seed) { let mut rng = make_seedable_rng(seed); let order_id = OrderId::random_using(&mut rng);