Add tests and minor fixes

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)?;

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()
+            ))
         );
     }
 

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,

chainstate/src/detail/error_classification.rs

@@ -840,6 +840,7 @@ impl BlockProcessingErrorClassification for orders_accounting::Error {
             | Error::FillOrderChangeLeft(_)
             | Error::CurrencyMismatch
             | Error::OrderOverflow(_)
+            | Error::OrderOverbid(_, _, _)
             | Error::AttemptedCancelNonexistingOrderData(_)
             | Error::AttemptedCancelNonexistingAskBalance(_)
             | Error::AttemptedCancelNonexistingGiveBalance(_)

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())]

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,

orders-accounting/src/cache.rs

@@ -84,7 +84,7 @@ impl<P: OrdersAccountingView> OrdersAccountingCache<P> {
         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<P: OrdersAccountingView> 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),
         }
     }

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,23 +39,25 @@ 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),
     #[error("Coin type mismatch")]
     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,

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)]

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);