Repository: rust-bitcoin/rust-bitcoin
Pull request #3769 (2024-12-14 16:11:23)

Change method return type for to_unsigned()
Any SignedAmount can now be cast to Amount since the range is the same.  Specifically, the range for SignedAmount is (- 21 million, 21 million) while the range for Amount is (0, 21 million).  Therefore any value from Amount can be cast to a SignedAmount and it will work.  Note it's not the same and still requires checking when going from SignedAmount to Amount since Amount can't handle the negative range.

coveralls: ## Pull Request Test Coverage Report for [Build 12331301464](https://coveralls.io/builds/71354161)


### Details

* **7** of **7**   **(100.0%)**  changed or added relevant lines in **3** files are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage decreased (**-0.007%**) to **82.909%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/71354161/badge)](https://coveralls.io/builds/71354161) |
| :-- | --: |
| Change from base [Build 12330687398](https://coveralls.io/builds/71353849): |  -0.007% |
| Covered Lines: | 20273 |
| Relevant Lines: | 24452 |

---
##### 💛  - [Coveralls](https://coveralls.io)

yancyribbens: There is still a check required when going from signed to unsigned.  Marking as draft until I update it.
yancyribbens: Apologies for the noise.  This is now ready for review.
yancyribbens: rebased
apoelstra: In 84632b6b89e15d49b020305017742f470f17effe:

Need comment on the new cast.
yancyribbens: > Need comment on the new cast.

Updated comment.  Also, since we are allowing a loophole in the invarient, this comments on that and is based on the commit that adds `from_sat_unchecked`.  There's no reason we can't introduce that as a smaller PR for easy review.

```rust
    /// Converts to a signed amount.
    ///
    /// # Panics
    ///
    /// If Amount violates the invariant.
    pub fn to_signed(self) -> SignedAmount {
        SignedAmount::from_sat_unchecked(self.to_sat() as i64)
    }
```
yancyribbens: should be SighnedAmount not Amount.  Will update.
yancyribbens: ~should be SighnedAmount not Amount. Will update.~

The original is correct. 

yancyribbens: I used Tobin's copy comment for `_unchecked` since he disagreed with mine.
apoelstra: There is still no comment on the cast, and the doccomment is confusing. "Panics if `Amount` violates the invariant"? **We do not and should not attempt to document what happens if the user uses unchecked methods to violate invariants.** The point of invariants is that they are invariant.
yancyribbens: > There is still no comment on the cast, and the doccomment is confusing.

Sorry for my confusion.  Can you point me to what you mean by comment the cast?  I had thought you meant the `to_signed()` method needs commenting (which performs the cast).  I'll undo that comment since we don't need to comment on the invarient after all.
apoelstra: >Sorry for my confusion. Can you point me to what you mean by comment the cast?

I mean that, on the same line as `as i64`, there needs to be a comment explaining why it's ok to use a truncating cast. This way when we grep for casts we can see that it's justified.
yancyribbens: > I mean that, on the same line as as i64, there needs to be a comment explaining why it's ok to use a truncating cast. This way when we grep for casts we can see that it's justified.

Got it.  Thanks for the clarification.  The format will complain if it's on the same line, so I placed it above, unless  we really really need it on the same line.
yancyribbens: I see now that before this has a rustfmt skip just to keep it on the same line?  I guess I should follow suit with that.
tcharding: This doesn't make the `amount` module material worse so I'm fine for it to merge. I'm not acking because this PR has problems but I don't want to invest the time to explain them. I'm going over the whole module anyways in #3794.
yancyribbens: > @yancyribbens I basically have no faith that your changes don't introduce bugs or are wrong in some other way that I didn't think out.

@tcharding To be fair, I don't have much faith in the quality of your reviews, which I think results in a lot of friction over the last few years.  There are maintainers on this project that when they Ack something, I'm confident they understand what they are Acking.  I've observed more than a few occasions where you Ack things you don't understand, and in one case, it came back to bite us both.  Making a mistake and missing something is unavoidable which is why a two ack policy is nice, but if the second ack is just an echo of the first, that doesn't provide much of a safe-guard.  I've seen many bugs of yours get caught by reviews, imagine how you would have felt if it was missed by a reviewer because they didn't understand what they were reviewing well enough.  This is one reason I choose to not Ack things unless I'm really sure, and since I'm not a maintainer, I consider it to be misleading even if I do go around typing Ack things I am confident on.

> To help me gain faith please do tiny patches and fully explain everything you do. 

Great idea and I encourage you lead the way and do the same.  Personally I try to make respectable size PRs and reviewable size commits to reduce the possibility of things being missed.

I hope we can understand each other and try to build trust in each others work.
yancyribbens: Here is the commit message you say give you no confidence:

```
What: Add unchecked variant

Why: This variant is supplied in cases where a Result type does not need to
be handled.
```

What exactly is it you disagree with so fervently.  Unchecked is added because we don't want to deal with the return type and perform `unwrap()`.  While I'm sure there's a lot more that could be said, possibly in a more accurate way, this isn't something that I would chastise as obviously wrong.  If looked at with the corresponding diff, what could be improved in your opinion?
yancyribbens: > this PR removes TryFrom but does not add From and does not mention either of these things in the git log)

This is a good call out that From should be added since TryFrom is removed.  However, I don't think that it's a dangerous or egregious miss and could be added as a followup.  Thank you for point this out though and I agree there could be something in the commit log about it as well.

Patch:
diff --git a/api/units/all-features.txt b/api/units/all-features.txt
index 6217193e..fe350990 100644
--- a/api/units/all-features.txt
+++ b/api/units/all-features.txt
@@ -145,6 +145,7 @@ impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Height
 impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Time
 impl core::cmp::PartialOrd for bitcoin_units::weight::Weight
 impl core::convert::AsRef<core::num::error::ParseIntError> for bitcoin_units::parse::ParseIntError
+impl core::convert::From<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseAmountError
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseError
 impl core::convert::From<bitcoin_units::amount::InvalidCharacterError> for bitcoin_units::amount::ParseAmountError
@@ -198,7 +199,6 @@ impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::relative::Height
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::relative::Time
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::weight::Weight
-impl core::convert::TryFrom<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::TryFrom<bitcoin_units::SignedAmount> for bitcoin_units::Amount
 impl core::convert::TryFrom<bitcoin_units::block::BlockHeight> for bitcoin_units::locktime::absolute::Height
 impl core::convert::TryFrom<bitcoin_units::block::BlockInterval> for bitcoin_units::locktime::relative::Height
@@ -707,8 +707,8 @@ pub const bitcoin_units::weight::Weight::WITNESS_SCALE_FACTOR: u64
 pub const bitcoin_units::weight::Weight::ZERO: bitcoin_units::weight::Weight
 pub const fn bitcoin_units::Amount::checked_add(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_div(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
-pub const fn bitcoin_units::Amount::checked_div_by_weight_ceil(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
-pub const fn bitcoin_units::Amount::checked_div_by_weight_floor(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
+pub const fn bitcoin_units::Amount::checked_div_by_weight_ceil(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
+pub const fn bitcoin_units::Amount::checked_div_by_weight_floor(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
 pub const fn bitcoin_units::Amount::checked_mul(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_rem(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_sub(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
@@ -723,6 +723,7 @@ pub const fn bitcoin_units::SignedAmount::checked_rem(self, rhs: i64) -> core::o
 pub const fn bitcoin_units::SignedAmount::checked_sub(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
 pub const fn bitcoin_units::SignedAmount::from_int_btc_const(whole_bitcoin: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::from_sat(satoshi: i64) -> bitcoin_units::SignedAmount
+pub const fn bitcoin_units::SignedAmount::from_sat_unchecked(satoshi: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::to_sat(self) -> i64
 pub const fn bitcoin_units::block::BlockHeight::from_u32(inner: u32) -> Self
 pub const fn bitcoin_units::block::BlockHeight::to_u32(self) -> u32
@@ -731,7 +732,7 @@ pub const fn bitcoin_units::block::BlockInterval::to_u32(self) -> u32
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_add(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_div(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul(self, rhs: u64) -> core::option::Option<Self>
-pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
+pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_sub(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kvb(sat_kvb: u64) -> Self
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kwu(sat_kwu: u64) -> Self
@@ -830,7 +831,7 @@ pub fn bitcoin_units::Amount::sum<I: core::iter::traits::iterator::Iterator<Item
 pub fn bitcoin_units::Amount::sum<I>(iter: I) -> Self where I: core::iter::traits::iterator::Iterator<Item = &'a bitcoin_units::Amount>
 pub fn bitcoin_units::Amount::to_btc(self) -> f64
 pub fn bitcoin_units::Amount::to_float_in(self, denom: bitcoin_units::amount::Denomination) -> f64
-pub fn bitcoin_units::Amount::to_signed(self) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
+pub fn bitcoin_units::Amount::to_signed(self) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::Amount::to_string_in(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::Amount::to_string_with_denomination(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::Amount::try_from(value: bitcoin_units::SignedAmount) -> core::result::Result<Self, Self::Error>
@@ -855,6 +856,7 @@ pub fn bitcoin_units::SignedAmount::div(self, rhs: i64) -> Self::Output
 pub fn bitcoin_units::SignedAmount::div_assign(&mut self, rhs: i64)
 pub fn bitcoin_units::SignedAmount::eq(&self, other: &bitcoin_units::SignedAmount) -> bool
 pub fn bitcoin_units::SignedAmount::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_units::SignedAmount::from(value: bitcoin_units::Amount) -> Self
 pub fn bitcoin_units::SignedAmount::from_btc(btc: f64) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::ParseAmountError>
 pub fn bitcoin_units::SignedAmount::from_float_in(value: f64, denom: bitcoin_units::amount::Denomination) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::ParseAmountError>
 pub fn bitcoin_units::SignedAmount::from_int_btc<T: core::convert::Into<i64>>(whole_bitcoin: T) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
@@ -889,7 +891,6 @@ pub fn bitcoin_units::SignedAmount::to_float_in(self, denom: bitcoin_units::amou
 pub fn bitcoin_units::SignedAmount::to_string_in(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::SignedAmount::to_string_with_denomination(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::SignedAmount::to_unsigned(self) -> core::result::Result<bitcoin_units::Amount, bitcoin_units::amount::OutOfRangeError>
-pub fn bitcoin_units::SignedAmount::try_from(value: bitcoin_units::Amount) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin_units::SignedAmount::type_prefix(_: private::Token) -> &'static str
 pub fn bitcoin_units::SignedAmount::unchecked_add(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::SignedAmount::unchecked_sub(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
@@ -1254,7 +1255,6 @@ pub type bitcoin_units::Amount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::Amount::Output = bitcoin_units::Amount
 pub type bitcoin_units::Amount::Output = bitcoin_units::fee_rate::FeeRate
 pub type bitcoin_units::SignedAmount::Err = bitcoin_units::amount::ParseError
-pub type bitcoin_units::SignedAmount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::SignedAmount::Output = bitcoin_units::SignedAmount
 pub type bitcoin_units::amount::Denomination::Err = bitcoin_units::amount::ParseDenominationError
 pub type bitcoin_units::block::BlockHeight::Err = bitcoin_units::parse::ParseIntError
diff --git a/api/units/alloc-only.txt b/api/units/alloc-only.txt
index 13e5cb27..f729d550 100644
--- a/api/units/alloc-only.txt
+++ b/api/units/alloc-only.txt
@@ -141,6 +141,7 @@ impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Height
 impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Time
 impl core::cmp::PartialOrd for bitcoin_units::weight::Weight
 impl core::convert::AsRef<core::num::error::ParseIntError> for bitcoin_units::parse::ParseIntError
+impl core::convert::From<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseAmountError
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseError
 impl core::convert::From<bitcoin_units::amount::InvalidCharacterError> for bitcoin_units::amount::ParseAmountError
@@ -194,7 +195,6 @@ impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::relative::Height
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::locktime::relative::Time
 impl core::convert::TryFrom<alloc::string::String> for bitcoin_units::weight::Weight
-impl core::convert::TryFrom<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::TryFrom<bitcoin_units::SignedAmount> for bitcoin_units::Amount
 impl core::convert::TryFrom<bitcoin_units::block::BlockHeight> for bitcoin_units::locktime::absolute::Height
 impl core::convert::TryFrom<bitcoin_units::block::BlockInterval> for bitcoin_units::locktime::relative::Height
@@ -658,8 +658,8 @@ pub const bitcoin_units::weight::Weight::WITNESS_SCALE_FACTOR: u64
 pub const bitcoin_units::weight::Weight::ZERO: bitcoin_units::weight::Weight
 pub const fn bitcoin_units::Amount::checked_add(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_div(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
-pub const fn bitcoin_units::Amount::checked_div_by_weight_ceil(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
-pub const fn bitcoin_units::Amount::checked_div_by_weight_floor(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
+pub const fn bitcoin_units::Amount::checked_div_by_weight_ceil(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
+pub const fn bitcoin_units::Amount::checked_div_by_weight_floor(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::fee_rate::FeeRate>
 pub const fn bitcoin_units::Amount::checked_mul(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_rem(self, rhs: u64) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::Amount::checked_sub(self, rhs: bitcoin_units::Amount) -> core::option::Option<bitcoin_units::Amount>
@@ -674,6 +674,7 @@ pub const fn bitcoin_units::SignedAmount::checked_rem(self, rhs: i64) -> core::o
 pub const fn bitcoin_units::SignedAmount::checked_sub(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
 pub const fn bitcoin_units::SignedAmount::from_int_btc_const(whole_bitcoin: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::from_sat(satoshi: i64) -> bitcoin_units::SignedAmount
+pub const fn bitcoin_units::SignedAmount::from_sat_unchecked(satoshi: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::to_sat(self) -> i64
 pub const fn bitcoin_units::block::BlockHeight::from_u32(inner: u32) -> Self
 pub const fn bitcoin_units::block::BlockHeight::to_u32(self) -> u32
@@ -682,7 +683,7 @@ pub const fn bitcoin_units::block::BlockInterval::to_u32(self) -> u32
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_add(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_div(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul(self, rhs: u64) -> core::option::Option<Self>
-pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
+pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_sub(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kvb(sat_kvb: u64) -> Self
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kwu(sat_kwu: u64) -> Self
@@ -771,7 +772,7 @@ pub fn bitcoin_units::Amount::sum<I: core::iter::traits::iterator::Iterator<Item
 pub fn bitcoin_units::Amount::sum<I>(iter: I) -> Self where I: core::iter::traits::iterator::Iterator<Item = &'a bitcoin_units::Amount>
 pub fn bitcoin_units::Amount::to_btc(self) -> f64
 pub fn bitcoin_units::Amount::to_float_in(self, denom: bitcoin_units::amount::Denomination) -> f64
-pub fn bitcoin_units::Amount::to_signed(self) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
+pub fn bitcoin_units::Amount::to_signed(self) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::Amount::to_string_in(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::Amount::to_string_with_denomination(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::Amount::try_from(value: bitcoin_units::SignedAmount) -> core::result::Result<Self, Self::Error>
@@ -791,6 +792,7 @@ pub fn bitcoin_units::SignedAmount::div(self, rhs: i64) -> Self::Output
 pub fn bitcoin_units::SignedAmount::div_assign(&mut self, rhs: i64)
 pub fn bitcoin_units::SignedAmount::eq(&self, other: &bitcoin_units::SignedAmount) -> bool
 pub fn bitcoin_units::SignedAmount::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_units::SignedAmount::from(value: bitcoin_units::Amount) -> Self
 pub fn bitcoin_units::SignedAmount::from_btc(btc: f64) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::ParseAmountError>
 pub fn bitcoin_units::SignedAmount::from_float_in(value: f64, denom: bitcoin_units::amount::Denomination) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::ParseAmountError>
 pub fn bitcoin_units::SignedAmount::from_int_btc<T: core::convert::Into<i64>>(whole_bitcoin: T) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
@@ -819,7 +821,6 @@ pub fn bitcoin_units::SignedAmount::to_float_in(self, denom: bitcoin_units::amou
 pub fn bitcoin_units::SignedAmount::to_string_in(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::SignedAmount::to_string_with_denomination(self, denom: bitcoin_units::amount::Denomination) -> alloc::string::String
 pub fn bitcoin_units::SignedAmount::to_unsigned(self) -> core::result::Result<bitcoin_units::Amount, bitcoin_units::amount::OutOfRangeError>
-pub fn bitcoin_units::SignedAmount::try_from(value: bitcoin_units::Amount) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin_units::SignedAmount::unchecked_add(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::SignedAmount::unchecked_sub(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::SignedAmount::unsigned_abs(self) -> bitcoin_units::Amount
@@ -1116,7 +1117,6 @@ pub type bitcoin_units::Amount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::Amount::Output = bitcoin_units::Amount
 pub type bitcoin_units::Amount::Output = bitcoin_units::fee_rate::FeeRate
 pub type bitcoin_units::SignedAmount::Err = bitcoin_units::amount::ParseError
-pub type bitcoin_units::SignedAmount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::SignedAmount::Output = bitcoin_units::SignedAmount
 pub type bitcoin_units::amount::Denomination::Err = bitcoin_units::amount::ParseDenominationError
 pub type bitcoin_units::block::BlockHeight::Err = bitcoin_units::parse::ParseIntError
diff --git a/api/units/no-features.txt b/api/units/no-features.txt
index 17a93af2..dfcb91f3 100644
--- a/api/units/no-features.txt
+++ b/api/units/no-features.txt
@@ -141,6 +141,7 @@ impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Height
 impl core::cmp::PartialOrd for bitcoin_units::locktime::relative::Time
 impl core::cmp::PartialOrd for bitcoin_units::weight::Weight
 impl core::convert::AsRef<core::num::error::ParseIntError> for bitcoin_units::parse::ParseIntError
+impl core::convert::From<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseAmountError
 impl core::convert::From<bitcoin_units::amount::InputTooLargeError> for bitcoin_units::amount::ParseError
 impl core::convert::From<bitcoin_units::amount::InvalidCharacterError> for bitcoin_units::amount::ParseAmountError
@@ -178,7 +179,6 @@ impl core::convert::TryFrom<&str> for bitcoin_units::locktime::absolute::Time
 impl core::convert::TryFrom<&str> for bitcoin_units::locktime::relative::Height
 impl core::convert::TryFrom<&str> for bitcoin_units::locktime::relative::Time
 impl core::convert::TryFrom<&str> for bitcoin_units::weight::Weight
-impl core::convert::TryFrom<bitcoin_units::Amount> for bitcoin_units::SignedAmount
 impl core::convert::TryFrom<bitcoin_units::SignedAmount> for bitcoin_units::Amount
 impl core::convert::TryFrom<bitcoin_units::block::BlockHeight> for bitcoin_units::locktime::absolute::Height
 impl core::convert::TryFrom<bitcoin_units::block::BlockInterval> for bitcoin_units::locktime::relative::Height
@@ -656,6 +656,7 @@ pub const fn bitcoin_units::SignedAmount::checked_rem(self, rhs: i64) -> core::o
 pub const fn bitcoin_units::SignedAmount::checked_sub(self, rhs: bitcoin_units::SignedAmount) -> core::option::Option<bitcoin_units::SignedAmount>
 pub const fn bitcoin_units::SignedAmount::from_int_btc_const(whole_bitcoin: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::from_sat(satoshi: i64) -> bitcoin_units::SignedAmount
+pub const fn bitcoin_units::SignedAmount::from_sat_unchecked(satoshi: i64) -> bitcoin_units::SignedAmount
 pub const fn bitcoin_units::SignedAmount::to_sat(self) -> i64
 pub const fn bitcoin_units::block::BlockHeight::from_u32(inner: u32) -> Self
 pub const fn bitcoin_units::block::BlockHeight::to_u32(self) -> u32
@@ -664,7 +665,7 @@ pub const fn bitcoin_units::block::BlockInterval::to_u32(self) -> u32
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_add(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_div(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul(self, rhs: u64) -> core::option::Option<Self>
-pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, weight: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
+pub const fn bitcoin_units::fee_rate::FeeRate::checked_mul_by_weight(self, rhs: bitcoin_units::weight::Weight) -> core::option::Option<bitcoin_units::Amount>
 pub const fn bitcoin_units::fee_rate::FeeRate::checked_sub(self, rhs: u64) -> core::option::Option<Self>
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kvb(sat_kvb: u64) -> Self
 pub const fn bitcoin_units::fee_rate::FeeRate::from_sat_per_kwu(sat_kwu: u64) -> Self
@@ -749,7 +750,7 @@ pub fn bitcoin_units::Amount::sub_assign(&mut self, rhs: &bitcoin_units::Amount)
 pub fn bitcoin_units::Amount::sub_assign(&mut self, rhs: bitcoin_units::Amount)
 pub fn bitcoin_units::Amount::sum<I: core::iter::traits::iterator::Iterator<Item = Self>>(iter: I) -> Self
 pub fn bitcoin_units::Amount::sum<I>(iter: I) -> Self where I: core::iter::traits::iterator::Iterator<Item = &'a bitcoin_units::Amount>
-pub fn bitcoin_units::Amount::to_signed(self) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
+pub fn bitcoin_units::Amount::to_signed(self) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::Amount::try_from(value: bitcoin_units::SignedAmount) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin_units::Amount::unchecked_add(self, rhs: bitcoin_units::Amount) -> bitcoin_units::Amount
 pub fn bitcoin_units::Amount::unchecked_sub(self, rhs: bitcoin_units::Amount) -> bitcoin_units::Amount
@@ -767,6 +768,7 @@ pub fn bitcoin_units::SignedAmount::div(self, rhs: i64) -> Self::Output
 pub fn bitcoin_units::SignedAmount::div_assign(&mut self, rhs: i64)
 pub fn bitcoin_units::SignedAmount::eq(&self, other: &bitcoin_units::SignedAmount) -> bool
 pub fn bitcoin_units::SignedAmount::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
+pub fn bitcoin_units::SignedAmount::from(value: bitcoin_units::Amount) -> Self
 pub fn bitcoin_units::SignedAmount::from_int_btc<T: core::convert::Into<i64>>(whole_bitcoin: T) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::OutOfRangeError>
 pub fn bitcoin_units::SignedAmount::from_str(s: &str) -> core::result::Result<Self, Self::Err>
 pub fn bitcoin_units::SignedAmount::from_str_in(s: &str, denom: bitcoin_units::amount::Denomination) -> core::result::Result<bitcoin_units::SignedAmount, bitcoin_units::amount::ParseAmountError>
@@ -789,7 +791,6 @@ pub fn bitcoin_units::SignedAmount::sub_assign(&mut self, rhs: bitcoin_units::Si
 pub fn bitcoin_units::SignedAmount::sum<I: core::iter::traits::iterator::Iterator<Item = Self>>(iter: I) -> Self
 pub fn bitcoin_units::SignedAmount::sum<I>(iter: I) -> Self where I: core::iter::traits::iterator::Iterator<Item = &'a bitcoin_units::SignedAmount>
 pub fn bitcoin_units::SignedAmount::to_unsigned(self) -> core::result::Result<bitcoin_units::Amount, bitcoin_units::amount::OutOfRangeError>
-pub fn bitcoin_units::SignedAmount::try_from(value: bitcoin_units::Amount) -> core::result::Result<Self, Self::Error>
 pub fn bitcoin_units::SignedAmount::unchecked_add(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::SignedAmount::unchecked_sub(self, rhs: bitcoin_units::SignedAmount) -> bitcoin_units::SignedAmount
 pub fn bitcoin_units::SignedAmount::unsigned_abs(self) -> bitcoin_units::Amount
@@ -1070,7 +1071,6 @@ pub type bitcoin_units::Amount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::Amount::Output = bitcoin_units::Amount
 pub type bitcoin_units::Amount::Output = bitcoin_units::fee_rate::FeeRate
 pub type bitcoin_units::SignedAmount::Err = bitcoin_units::amount::ParseError
-pub type bitcoin_units::SignedAmount::Error = bitcoin_units::amount::OutOfRangeError
 pub type bitcoin_units::SignedAmount::Output = bitcoin_units::SignedAmount
 pub type bitcoin_units::amount::Denomination::Err = bitcoin_units::amount::ParseDenominationError
 pub type bitcoin_units::block::BlockHeight::Err = bitcoin_units::parse::ParseIntError
diff --git a/base58/src/lib.rs b/base58/src/lib.rs
index df2c1510..b5b977eb 100644
--- a/base58/src/lib.rs
+++ b/base58/src/lib.rs
@@ -337,7 +337,7 @@ pub fn bench_encode_check_50(bh: &mut Bencher) {
 
     #[bench]
     pub fn bench_encode_check_xpub(bh: &mut Bencher) {
-        let data: alloc::vec::Vec<_> = (0u8..78).collect(); // length of xpub
+        let data: alloc::vec::Vec<_> = (0u8..78).collect(); // lenght of xpub
 
         bh.iter(|| {
             let r = super::encode_check(&data);
diff --git a/bitcoin/src/blockdata/transaction.rs b/bitcoin/src/blockdata/transaction.rs
index 84c53de7..5dea49ba 100644
--- a/bitcoin/src/blockdata/transaction.rs
+++ b/bitcoin/src/blockdata/transaction.rs
@@ -790,8 +790,8 @@ pub fn effective_value(
     value: Amount,
 ) -> Option<SignedAmount> {
     let weight = satisfaction_weight.checked_add(TX_IN_BASE_WEIGHT)?;
-    let signed_input_fee = fee_rate.checked_mul_by_weight(weight)?.to_signed().ok()?;
-    value.to_signed().ok()?.checked_sub(signed_input_fee)
+    let signed_input_fee = fee_rate.checked_mul_by_weight(weight)?.to_signed();
+    value.to_signed().checked_sub(signed_input_fee)
 }
 
 /// Predicts the weight of a to-be-constructed transaction.
@@ -1673,7 +1673,7 @@ fn effective_value_happy_path() {
 
         // 10 sat/kwu * (204wu + BASE_WEIGHT) = 4 sats
         let expected_fee = "4 sats".parse::<SignedAmount>().unwrap();
-        let expected_effective_value = value.to_signed().unwrap() - expected_fee;
+        let expected_effective_value = value.to_signed() - expected_fee;
         assert_eq!(effective_value, expected_effective_value);
     }
 
diff --git a/bitcoin/src/merkle_tree/mod.rs b/bitcoin/src/merkle_tree/mod.rs
index 82801fdd..b656ce7e 100644
--- a/bitcoin/src/merkle_tree/mod.rs
+++ b/bitcoin/src/merkle_tree/mod.rs
@@ -41,7 +41,7 @@
 /// Other Merkle trees in Bitcoin, such as those used in Taproot commitments,
 /// do not use this algorithm and cannot use this trait.
 pub trait MerkleNode: Copy {
-    /// The hash (TXID or WTXID) of a transaction in the tree.
+    /// The hash (TXID or WTXID) of a transaciton in the tree.
     type Leaf: TxIdentifier;
 
     /// Convert a hash to a leaf node of the tree.
diff --git a/bitcoin/src/psbt/mod.rs b/bitcoin/src/psbt/mod.rs
index 098e853e..071b5afa 100644
--- a/bitcoin/src/psbt/mod.rs
+++ b/bitcoin/src/psbt/mod.rs
@@ -712,15 +712,15 @@ fn output_type(&self, input_index: usize) -> Result<OutputType, SignError> {
     /// - [`Error::NegativeFee`] if calculated value is negative.
     /// - [`Error::FeeOverflow`] if an integer overflow occurs.
     pub fn fee(&self) -> Result<Amount, Error> {
-        let mut inputs = Amount::ZERO;
+        let mut inputs: u64 = 0;
         for utxo in self.iter_funding_utxos() {
-            inputs = inputs.checked_add(utxo?.value).ok_or(Error::FeeOverflow)?;
+            inputs = inputs.checked_add(utxo?.value.to_sat()).ok_or(Error::FeeOverflow)?;
         }
-        let mut outputs = Amount::ZERO;
+        let mut outputs: u64 = 0;
         for out in &self.unsigned_tx.output {
-            outputs = outputs.checked_add(out.value).ok_or(Error::FeeOverflow)?;
+            outputs = outputs.checked_add(out.value.to_sat()).ok_or(Error::FeeOverflow)?;
         }
-        inputs.checked_sub(outputs).ok_or(Error::NegativeFee)
+        inputs.checked_sub(outputs).map(Amount::from_sat).ok_or(Error::NegativeFee)
     }
 }
 
diff --git a/chacha20_poly1305/src/chacha20.rs b/chacha20_poly1305/src/chacha20.rs
index 0a73ca36..51d0e4b8 100644
--- a/chacha20_poly1305/src/chacha20.rs
+++ b/chacha20_poly1305/src/chacha20.rs
@@ -45,7 +45,7 @@ impl Nonce {
 ///
 /// This type is attempting to be as close as possible to the experimental [`core::simd::u32x4`]
 /// which at this time is feature gated and well beyond the project's MSRV. But ideally
-/// an easy transition can be made in the future.
+/// an easy transistion can be made in the future.
 ///
 /// A few SIMD relevant design choices:
 ///    * Heavy use of inline functions to help the compiler recognize vectorizable sections.
diff --git a/hashes/CHANGELOG.md b/hashes/CHANGELOG.md
index bd9aed73..ea31a9be 100644
--- a/hashes/CHANGELOG.md
+++ b/hashes/CHANGELOG.md
@@ -6,7 +6,7 @@
 
 # 0.15.0 - 2024-10-16
 
-This release is massive. The biggest visible changes are to the `Hash` trait, which has mostly been replaced
+This release is massive. The biggest visable changes are to the `Hash` trait, which has mostly been replaced
 by inherent functions. You should not need to import it at all anymore for normal usage. Check out how we are
 using `hashes` in `rust-bitcoin` to see an example. Enjoy!
 
@@ -52,7 +52,7 @@ using `hashes` in `rust-bitcoin` to see an example. Enjoy!
 
 * Bump MSRV to Rust version 1.56.1 [#2188](https://github.com/rust-bitcoin/rust-bitcoin/pull/2188)
 
-## API improvements
+## API improvemnts
 
 * Add support for SHA384 [#2538](https://github.com/rust-bitcoin/rust-bitcoin/pull/2538)
 * Make from_hex inherent for byte-like types [#2491](https://github.com/rust-bitcoin/rust-bitcoin/pull/2491)
diff --git a/units/src/amount/signed.rs b/units/src/amount/signed.rs
index 2faa7e45..3c9df7f6 100644
--- a/units/src/amount/signed.rs
+++ b/units/src/amount/signed.rs
@@ -83,6 +83,15 @@ pub const fn from_sat(satoshi: i64) -> SignedAmount { SignedAmount(satoshi) }
     /// ```
     pub const fn to_sat(self) -> i64 { self.0 }
 
+    /// Constructs a new [`SignedAmount`] with satoshi precision and the given number of satoshis.
+    ///
+    /// Caller to guarantee that `satoshi` is within valid range.
+    ///
+    /// See [`Self::MIN`] and [`Self::MAX_MONEY`].
+    pub const fn from_sat_unchecked(satoshi: i64) -> SignedAmount {
+        SignedAmount(satoshi)
+    }
+
     /// Converts from a value expressing a decimal number of bitcoin to a [`SignedAmount`].
     ///
     /// # Errors
@@ -557,10 +566,11 @@ fn from_str(s: &str) -> Result<Self, Self::Err> {
     }
 }
 
-impl TryFrom<Amount> for SignedAmount {
-    type Error = OutOfRangeError;
-
-    fn try_from(value: Amount) -> Result<Self, Self::Error> { value.to_signed() }
+impl From<Amount> for SignedAmount {
+    fn from(value: Amount) -> Self {
+        let v = value.to_sat() as i64; // Cast ok, signed amount and amount share positive range.
+        SignedAmount::from_sat_unchecked(v)
+    }
 }
 
 impl core::iter::Sum for SignedAmount {
diff --git a/units/src/amount/unsigned.rs b/units/src/amount/unsigned.rs
index e7b21418..718c6e47 100644
--- a/units/src/amount/unsigned.rs
+++ b/units/src/amount/unsigned.rs
@@ -369,8 +369,8 @@ pub const fn checked_div(self, rhs: u64) -> Option<Amount> {
     /// ```
     #[cfg(feature = "alloc")]
     #[must_use]
-    pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option<FeeRate> {
-        let wu = weight.to_wu();
+    pub const fn checked_div_by_weight_ceil(self, rhs: Weight) -> Option<FeeRate> {
+        let wu = rhs.to_wu();
         // No `?` operator in const context.
         if let Some(sats) = self.0.checked_mul(1_000) {
             if let Some(wu_minus_one) = wu.checked_sub(1) {
@@ -392,10 +392,10 @@ pub const fn checked_div_by_weight_ceil(self, weight: Weight) -> Option<FeeRate>
     /// Returns [`None`] if overflow occurred.
     #[cfg(feature = "alloc")]
     #[must_use]
-    pub const fn checked_div_by_weight_floor(self, weight: Weight) -> Option<FeeRate> {
+    pub const fn checked_div_by_weight_floor(self, rhs: Weight) -> Option<FeeRate> {
         // No `?` operator in const context.
         match self.0.checked_mul(1_000) {
-            Some(res) => match res.checked_div(weight.to_wu()) {
+            Some(res) => match res.checked_div(rhs.to_wu()) {
                 Some(fee_rate) => Some(FeeRate::from_sat_per_kwu(fee_rate)),
                 None => None,
             },
@@ -438,17 +438,9 @@ pub fn unchecked_add(self, rhs: Amount) -> Amount { Self(self.0 + rhs.0) }
     pub fn unchecked_sub(self, rhs: Amount) -> Amount { Self(self.0 - rhs.0) }
 
     /// Converts to a signed amount.
-    ///
-    /// # Errors
-    ///
-    /// If the amount is too big.
     #[rustfmt::skip] // Moves code comments to the wrong line.
-    pub fn to_signed(self) -> Result<SignedAmount, OutOfRangeError> {
-        if self.to_sat() > SignedAmount::MAX.to_sat() as u64 { // Cast ok, signed max is positive and fits in u64.
-            Err(OutOfRangeError::too_big(true))
-        } else {
-            Ok(SignedAmount::from_sat(self.to_sat() as i64)) // Cast ok, checked not too big above.
-        }
+    pub fn to_signed(self) -> SignedAmount {
+        SignedAmount::from_sat_unchecked(self.to_sat() as i64) // Cast ok, signed amount and amount share positive range.
     }
 
     /// Checks if the amount is below the maximum value. Returns `None` if it is above.
diff --git a/units/src/fee_rate.rs b/units/src/fee_rate.rs
index 2483bd09..3429aa44 100644
--- a/units/src/fee_rate.rs
+++ b/units/src/fee_rate.rs
@@ -108,9 +108,9 @@ pub const fn checked_div(self, rhs: u64) -> Option<Self> {
     ///
     /// [`None`] is returned if an overflow occurred.
     #[must_use]
-    pub const fn checked_mul_by_weight(self, weight: Weight) -> Option<Amount> {
+    pub const fn checked_mul_by_weight(self, rhs: Weight) -> Option<Amount> {
         // No `?` operator in const context.
-        match self.0.checked_mul(weight.to_wu()) {
+        match self.0.checked_mul(rhs.to_wu()) {
             Some(mul_res) => match mul_res.checked_add(999) {
                 Some(add_res) => Some(Amount::from_sat(add_res / 1000)),
                 None => None,
@@ -121,7 +121,7 @@ pub const fn checked_mul_by_weight(self, weight: Weight) -> Option<Amount> {
 
     /// Checked addition.
     ///
-    /// Computes `self + rhs` returning [`None`] if overflow occurred.
+    /// Computes `self + rhs` returning [`None`] if overflow occured.
     #[must_use]
     pub const fn checked_add(self, rhs: u64) -> Option<Self> {
         // No `map()` in const context.
@@ -133,7 +133,7 @@ pub const fn checked_add(self, rhs: u64) -> Option<Self> {
 
     /// Checked subtraction.
     ///
-    /// Computes `self - rhs` returning [`None`] if overflow occurred.
+    /// Computes `self - rhs` returning [`None`] if overflow occured.
     #[must_use]
     pub const fn checked_sub(self, rhs: u64) -> Option<Self> {
         // No `map()` in const context.


Test patch:
diff --git a/units/src/amount/tests.rs b/units/src/amount/tests.rs
index c4f6ea9f..a7f08efb 100644
--- a/units/src/amount/tests.rs
+++ b/units/src/amount/tests.rs
@@ -71,13 +71,6 @@ fn from_int_btc() {
     assert_eq!(Amount::from_sat(200_000_000), amt);
 }
 
-#[test]
-fn test_signed_amount_try_from_amount() {
-    let ua_positive = Amount::from_sat(123);
-    let sa_positive = SignedAmount::try_from(ua_positive).unwrap();
-    assert_eq!(sa_positive, SignedAmount::from_sat(123));
-}
-
 #[test]
 fn test_amount_try_from_signed_amount() {
     let sa_positive = SignedAmount::from_sat(123);
@@ -473,12 +466,11 @@ fn test_unsigned_signed_conversion() {
     let ua = Amount::from_sat;
     let max_sats: u64 = Amount::MAX.to_sat();
 
-    assert_eq!(ua(max_sats).to_signed(), Ok(sa(max_sats as i64)));
-    assert_eq!(ua(i64::MAX as u64 + 1).to_signed(), Err(OutOfRangeError::too_big(true)));
+    assert_eq!(ua(max_sats).to_signed(), sa(max_sats as i64));
 
     assert_eq!(sa(max_sats as i64).to_unsigned(), Ok(ua(max_sats)));
 
-    assert_eq!(sa(max_sats as i64).to_unsigned().unwrap().to_signed(), Ok(sa(max_sats as i64)));
+    assert_eq!(sa(max_sats as i64).to_unsigned().unwrap().to_signed(), sa(max_sats as i64));
 }
 
 #[test]