Merge #605: Change fee_delta to SignedAmount

9bcbdb3 Change fee_delta to SignedAmount (Jamil Lambert, PhD)

Pull request description:

  `fee_delta` can be negative.

  Change it from `Amount` to `SignedAmount` and update the test to confirm that a negative value is accepted.

ACKs for top commit:
  tcharding:
    ACK 9bcbdb3

Tree-SHA512: 22a5bc004e0f8f4c67d8786754366a3e686a8c4e69c0a57d446d2a0480951a47e36e41f47a2d6d6a94f79c7cc1a70b78fa3c79e03278405a3ac99527930bff8e

Author: tcharding

integration_test/tests/mining.rs

@@ -71,8 +71,28 @@ fn mining__get_network_hash_ps() {
 fn mining__get_prioritised_transactions() {
     let node = BitcoinD::with_wallet(Wallet::Default, &[]);
     node.fund_wallet();
-
-    let _ = node.client.get_prioritised_transactions().expect("getprioritisedtransactions");
+    let (_address, txid) = node.create_mempool_transaction();
+
+    let fee_delta = SignedAmount::from_sat(-10_000);
+    node.client.prioritise_transaction(&txid, fee_delta).expect("prioritisetransaction");
+
+    let json: GetPrioritisedTransactions =
+        node.client.get_prioritised_transactions().expect("getprioritisedtransactions");
+    let model: Result<mtype::GetPrioritisedTransactions, bitcoin::hex::HexToArrayError> =
+        json.into_model();
+    let model = model.unwrap();
+
+    let prioritised_tx = model.0.get(&txid).expect("prioritised transaction should be present");
+    assert_eq!(prioritised_tx.fee_delta, fee_delta);
+
+    // modified_fee is only returned in v27 and above
+    #[cfg(not(feature = "v26_and_below"))]
+    {
+        let transaction = node.client.get_transaction(txid).expect("gettransaction");
+        let transaction = transaction.into_model().unwrap();
+        let base_fee = -transaction.fee.expect("transaction fee should be present");
+        assert_eq!(prioritised_tx.modified_fee, Some(base_fee + fee_delta));
+    }
 }
 
 #[test]

types/src/model/mining.rs

@@ -8,8 +8,8 @@
 use std::collections::BTreeMap;
 
 use bitcoin::{
-    block, Amount, BlockHash, CompactTarget, FeeRate, SignedAmount, Target, Transaction, Txid,
-    Weight, Wtxid,
+    block, BlockHash, CompactTarget, FeeRate, SignedAmount, Target, Transaction, Txid, Weight,
+    Wtxid,
 };
 use serde::{Deserialize, Serialize};
 
@@ -151,9 +151,11 @@ pub struct GetPrioritisedTransactions(pub BTreeMap<Txid, PrioritisedTransaction>
 #[derive(Clone, Debug, PartialEq, Deserialize, Serialize)]
 pub struct PrioritisedTransaction {
     /// Transaction fee delta in satoshis.
-    pub fee_delta: Amount,
+    #[serde(with = "bitcoin::amount::serde::as_sat")]
+    pub fee_delta: SignedAmount,
     /// Whether this transaction is currently in mempool.
     pub in_mempool: bool,
     /// Modified fee in satoshis. Only returned if in_mempool=true.
-    pub modified_fee: Option<Amount>,
+    #[serde(with = "bitcoin::amount::serde::as_sat::opt")]
+    pub modified_fee: Option<SignedAmount>,
 }

types/src/v26/mining.rs

@@ -6,7 +6,7 @@
 
 use std::collections::BTreeMap;
 
-use bitcoin::{hex, Amount, Txid};
+use bitcoin::{hex, SignedAmount, Txid};
 use serde::{Deserialize, Serialize};
 
 use crate::model;
@@ -49,7 +49,7 @@ impl PrioritisedTransaction {
     /// Converts version specific type to a version nonspecific, more strongly typed type.
     pub fn into_model(self) -> model::PrioritisedTransaction {
         model::PrioritisedTransaction {
-            fee_delta: Amount::from_sat(self.fee_delta as u64),
+            fee_delta: SignedAmount::from_sat(self.fee_delta),
             in_mempool: self.in_mempool,
             modified_fee: None,
         }

types/src/v27/mining.rs

@@ -6,7 +6,7 @@
 
 use std::collections::BTreeMap;
 
-use bitcoin::{hex, Amount, Txid};
+use bitcoin::{hex, SignedAmount, Txid};
 use serde::{Deserialize, Serialize};
 
 use crate::model;
@@ -32,7 +32,7 @@ pub struct PrioritisedTransaction {
     /// Whether this transaction is currently in mempool.
     pub in_mempool: bool,
     /// Modified fee in satoshis. Only returned if in_mempool=true.
-    pub modified_fee: Option<u64>,
+    pub modified_fee: Option<i64>,
 }
 
 impl GetPrioritisedTransactions {
@@ -51,9 +51,9 @@ impl PrioritisedTransaction {
     /// Converts version specific type to a version nonspecific, more strongly typed type.
     pub fn into_model(self) -> model::PrioritisedTransaction {
         model::PrioritisedTransaction {
-            fee_delta: Amount::from_sat(self.fee_delta as u64),
+            fee_delta: SignedAmount::from_sat(self.fee_delta),
             in_mempool: self.in_mempool,
-            modified_fee: self.modified_fee.map(Amount::from_sat),
+            modified_fee: self.modified_fee.map(SignedAmount::from_sat),
         }
     }
 }