Use bitcoin::Amount over u64

[bennyhodl]

Other bitcoin crates have migrated to using Amount over u64 with the migration to 0.32 which uses Amount. We should be aligned with the other bitcoin crates.

BDK migration - bitcoindevkit/bdk#1595

• dlc: use bitcoin::Amount
• dlc-trie: use bitcoin::Amount
• dlc-manager: use bitcoin::Amount
• dlc-messages: use bitcoin::Amount
• dlc-manager: use bitcoin::Amount
• test-utils: use bitcoin::Amount

[bennyhodl]

Thoughts on converting fee_rates, locktime, and weights as well?

[Tibo-lg]

LGTM, except a few nits that I would have fixed myself but I don't seem to be able to edit your branch somehow, I put you the patch here:

diff --git a/dlc-manager/src/contract/accepted_contract.rs b/dlc-manager/src/contract/accepted_contract.rs
index c9e7ad2..a4f71b8 100644
--- a/dlc-manager/src/contract/accepted_contract.rs
+++ b/dlc-manager/src/contract/accepted_contract.rs
@@ -1,5 +1,7 @@
 //! # AcceptedContract
 
+use crate::error::Error;
+
 use super::offered_contract::OfferedContract;
 use super::AdaptorInfo;
 use bitcoin::{Amount, SignedAmount, Transaction};
@@ -75,7 +77,7 @@ impl AcceptedContract {
     }
 
     /// Compute the profit and loss for this contract and an assciated cet index
-    pub fn compute_pnl(&self, cet: &Transaction) -> SignedAmount {
+    pub fn compute_pnl(&self, cet: &Transaction) -> Result<SignedAmount, Error> {
         let offer = &self.offered_contract;
         let party_params = if offer.is_offer_party {
             &offer.offer_params
@@ -95,7 +97,10 @@ impl AcceptedContract {
                 }
             })
             .unwrap_or(Amount::ZERO);
-        SignedAmount::from_sat(final_payout.to_sat() as i64 - collateral.to_sat() as i64)
+        Ok(final_payout
+            .to_signed()
+            .map_err(|_| Error::OutOfRangeError)?
+            - collateral.to_signed().map_err(|_| Error::OutOfRangeError)?)
     }
 }
 
@@ -113,11 +118,13 @@ mod tests {
         let accepted_contract: AcceptedContract = Readable::read(&mut Cursor::new(&buf)).unwrap();
         let cets = &accepted_contract.dlc_transactions.cets;
         assert_eq!(
-            accepted_contract.compute_pnl(&cets[0]),
+            accepted_contract.compute_pnl(&cets[0]).unwrap(),
             SignedAmount::from_sat(90000000)
         );
         assert_eq!(
-            accepted_contract.compute_pnl(&cets[cets.len() - 1]),
+            accepted_contract
+                .compute_pnl(&cets[cets.len() - 1])
+                .unwrap(),
             SignedAmount::from_sat(-11000000)
         );
     }
diff --git a/dlc-manager/src/contract/ser.rs b/dlc-manager/src/contract/ser.rs
index d05655a..d8fee84 100644
--- a/dlc-manager/src/contract/ser.rs
+++ b/dlc-manager/src/contract/ser.rs
@@ -136,7 +136,7 @@ impl_dlc_writeable!(ClosedContract, {
     (contract_id, writeable),
     (temporary_contract_id, writeable),
     (counter_party_id, writeable),
-    (pnl, i64)
+    (pnl, SignedAmount)
 });
 impl_dlc_writeable!(FailedAcceptContract, {(offered_contract, writeable), (accept_message, writeable), (error_message, string)});
 impl_dlc_writeable!(FailedSignContract, {(accepted_contract, writeable), (sign_message, writeable), (error_message, string)});
diff --git a/dlc-manager/src/error.rs b/dlc-manager/src/error.rs
index 9f0ff5c..fdd8e76 100644
--- a/dlc-manager/src/error.rs
+++ b/dlc-manager/src/error.rs
@@ -27,6 +27,8 @@ pub enum Error {
     DlcError(dlc::Error),
     /// An error occurred in the Secp library.
     SecpError(secp256k1_zkp::Error),
+    /// A computation was out of range
+    OutOfRangeError,
 }
 
 impl fmt::Display for Error {
@@ -43,6 +45,7 @@ impl fmt::Display for Error {
             Error::DlcError(ref e) => write!(f, "Dlc error {}", e),
             Error::OracleError(ref s) => write!(f, "Oracle error {}", s),
             Error::SecpError(_) => write!(f, "Secp error"),
+            Error::OutOfRangeError => write!(f, "Out of range error"),
         }
     }
 }
@@ -98,6 +101,7 @@ impl std::error::Error for Error {
             Error::OracleError(_) => None,
             Error::DlcError(e) => Some(e),
             Error::SecpError(e) => Some(e),
+            Error::OutOfRangeError => None,
         }
     }
 }
diff --git a/dlc-manager/src/manager.rs b/dlc-manager/src/manager.rs
index 84dd666..1722c76 100644
--- a/dlc-manager/src/manager.rs
+++ b/dlc-manager/src/manager.rs
@@ -755,7 +755,7 @@ where
                 pnl: contract
                     .signed_contract
                     .accepted_contract
-                    .compute_pnl(&contract.signed_cet),
+                    .compute_pnl(&contract.signed_cet)?,
             };
             self.store
                 .update_contract(&Contract::Closed(closed_contract))?;
@@ -800,7 +800,7 @@ where
 
         let closed_contract = ClosedContract {
             attestations: Some(attestations.to_vec()),
-            pnl: contract.accepted_contract.compute_pnl(&signed_cet),
+            pnl: contract.accepted_contract.compute_pnl(&signed_cet)?,
             signed_cet: Some(signed_cet),
             contract_id: contract.accepted_contract.get_contract_id(),
             temporary_contract_id: contract.accepted_contract.offered_contract.id,
@@ -883,7 +883,7 @@ where
         } else {
             Contract::Closed(ClosedContract {
                 attestations: None, // todo in some cases we can get the attestations from the closing tx
-                pnl: contract.accepted_contract.compute_pnl(&closing_tx),
+                pnl: contract.accepted_contract.compute_pnl(&closing_tx)?,
                 signed_cet: Some(closing_tx),
                 contract_id: contract.accepted_contract.get_contract_id(),
                 temporary_contract_id: contract.accepted_contract.offered_contract.id,
diff --git a/dlc-manager/src/payout_curve.rs b/dlc-manager/src/payout_curve.rs
index 9010012..411042a 100644
--- a/dlc-manager/src/payout_curve.rs
+++ b/dlc-manager/src/payout_curve.rs
@@ -167,6 +167,7 @@ trait Evaluable {
         total_collateral: Amount,
     ) -> Result<Amount, Error> {
         let payout_double = self.evaluate(outcome);
+        let total_collateral_sats = total_collateral.to_sat();
         if payout_double.is_sign_negative() || (payout_double != 0.0 && !payout_double.is_normal())
         {
             return Err(Error::InvalidParameters(format!(
@@ -175,7 +176,7 @@ trait Evaluable {
             )));
         }
 
-        if payout_double.round() > total_collateral.to_sat() as f64 {
+        if payout_double.round() > total_collateral_sats as f64 {
             return Err(Error::InvalidParameters(
                 "Computed payout is greater than total collateral".to_string(),
             ));
@@ -184,7 +185,7 @@ trait Evaluable {
         // Ensure that we never round over the total collateral.
         Ok(Amount::from_sat(u64::min(
             rounding_intervals.round(outcome, payout_double),
-            total_collateral.to_sat(),
+            total_collateral_sats,
         )))
     }
 
@@ -309,14 +310,14 @@ impl Evaluable for PolynomialPayoutCurvePiece {
         // Optimizations for constant and linear cases.
         if nb_points == 2 {
             let (left_point, right_point) = (&self.payout_points[0], &self.payout_points[1]);
+            let right_point_payout_sats = right_point.outcome_payout.to_sat() as f64;
+            let left_point_payout_sats = left_point.outcome_payout.to_sat() as f64;
             return if left_point.outcome_payout == right_point.outcome_payout {
-                right_point.outcome_payout.to_sat() as f64
+                right_point_payout_sats
             } else {
-                let slope = (right_point.outcome_payout.to_sat() as f64
-                    - left_point.outcome_payout.to_sat() as f64)
+                let slope = (right_point_payout_sats - left_point_payout_sats)
                     / (right_point.event_outcome - left_point.event_outcome) as f64;
-                (outcome - left_point.event_outcome) as f64 * slope
-                    + left_point.outcome_payout.to_sat() as f64
+                (outcome - left_point.event_outcome) as f64 * slope + left_point_payout_sats
             };
         }
 
diff --git a/dlc-messages/src/ser_impls.rs b/dlc-messages/src/ser_impls.rs
index f1e2245..1ba425d 100644
--- a/dlc-messages/src/ser_impls.rs
+++ b/dlc-messages/src/ser_impls.rs
@@ -568,17 +568,22 @@ pub fn read_i32<R: Read>(reader: &mut R) -> Result<i32, DecodeError> {
     let v: [u8; 4] = Readable::read(reader)?;
     Ok(i32::from_be_bytes(v))
 }
-/// Writes an `i64` value to the given writer.
-pub fn write_i64<W: Writer>(i: &i64, writer: &mut W) -> Result<(), ::lightning::io::Error> {
-    let i = i.to_be_bytes();
+/// Writes a `SignedAmount` value to the given writer.
+pub fn write_signed_amount<W: Writer>(
+    i: &SignedAmount,
+    writer: &mut W,
+) -> Result<(), ::lightning::io::Error> {
+    let i = i.to_sat().to_be_bytes();
     for b in i {
         b.write(writer)?;
     }
     Ok(())
 }
 
-/// Reads an `i64` value from the given reader.
-pub fn read_i64<R: ::lightning::io::Read>(reader: &mut R) -> Result<SignedAmount, DecodeError> {
+/// Reads a `SignedAmount` value from the given reader.
+pub fn read_signed_amount<R: ::lightning::io::Read>(
+    reader: &mut R,
+) -> Result<SignedAmount, DecodeError> {
     let mut v = [0u8; 8];
     for x in &mut v {
         *x = Readable::read(reader)?;
diff --git a/dlc-messages/src/ser_macros.rs b/dlc-messages/src/ser_macros.rs
index 94689a7..2c883c6 100644
--- a/dlc-messages/src/ser_macros.rs
+++ b/dlc-messages/src/ser_macros.rs
@@ -27,8 +27,8 @@ macro_rules! field_write {
     ($stream: expr, $field: expr, usize) => {
         $crate::ser_impls::write_usize(&$field, $stream)?;
     };
-    ($stream: expr, $field: expr, i64) => {
-        $crate::ser_impls::write_i64(&$field.to_sat(), $stream)?;
+    ($stream: expr, $field: expr, SignedAmount) => {
+        $crate::ser_impls::write_signed_amount(&$field, $stream)?;
     };
     ($stream: expr, $field: expr, {option_cb, $w_cb: expr, $r_cb: expr}) => {
         $crate::ser_impls::write_option_cb(&$field, $stream, &$w_cb)?;
@@ -65,8 +65,8 @@ macro_rules! field_read {
     ($stream: expr, usize) => {
         $crate::ser_impls::read_usize($stream)?
     };
-    ($stream: expr, i64) => {
-        $crate::ser_impls::read_i64($stream)?
+    ($stream: expr, SignedAmount) => {
+        $crate::ser_impls::read_signed_amount($stream)?
     };
     ($stream: expr, {option_cb, $w_cb: expr, $r_cb: expr}) => {
         $crate::ser_impls::read_option_cb($stream, &$r_cb)?

[Tibo-lg]

Thoughts on converting fee_rates, locktime, and weights as well?

I think it's nice to do it, I don't see any reason not to. Just need to be careful not to introduce bugs especially in the fee computation code.

[bennyhodl]

but I don't seem to be able to edit your branch somehow

Hm that's the second time that has happened now, I don't have any branch rules on

[yancyribbens]

Just a note that Amount is currently being updated to support a new MAX of 2100000000000000 sats (21 million btc) instead of u64::MAX. I browsed this PR quickly and didn't see that max being violated.

[Tibo-lg]

Thank you!