← Watch feed
Merge rust-bitcoin/rust-bitcoin#6790: Implement Rem<NonZero{I,U}64> for {Signed}Amount
Public commit record
91/100 · Strong
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6790: Implement Rem<NonZero{I,U}64> for {Signed}Amount
7ceb9f04cf33161590d9f8137dd606e2e23d498e Implement Rem<NonZero{I,U}64> for {Signed}Amount (Martin Habovstiak)
60e21d58adab5dec6fd83be1c997fa03b833c1bd Rename `amount/result.rs` to `amount/ops.rs` (Martin Habovstiak)
Pull request description:
`Rem` and `RemAssign` were already implemented for `u64` and `i64` and we also had `Div` and `DivAssign` for `NonZeroU64` and `NonZeroI64` but `Rem{Assign}` for `NonZero{U,I}64` were missing. This PR adds them along with a little cleanup.
ACKs for top commit:
apoelstra:
ACK 7ceb9f04cf33161590d9f8137dd606e2e23d498e; successfully ran local tests. LGTM. But lol that even stdlib's "nonzero" types are unusable without adding unreachable panic paths.
Tree-SHA512: c1c9513f6620ea4cc966be91e2529e790db17692c1c7d5d0360c8feb35392cd75d64255017ae2662f3ee6146d03198b98bf10d9b97dc1c47939a636d07465ea3
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This commit adds new ways to use the remainder operator (the '%' modulo operation) with Bitcoin amount types and Rust's non-zero integer types. It is a routine feature addition with no apparent security relevance. The code does not fix a vulnerability, change access controls, or introduce risky behavior beyond what was already present for similar operations.
Recommended action
No security action required. Review as normal code-quality/feature PR if desired.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The merge commit implements Rem<NonZeroU64> for Amount, Rem<NonZeroI64> for SignedAmount, and the corresponding RemAssign traits, plus Rem implementations for NumOpResult<Amount/SignedAmount>. It also renames amount/result.rs to amount/ops.rs and updates the module doc comment. The implementations delegate to the existing to_sat() % modulus.get() and from_sat(...).expect(...) patterns already used for Div with non-zero types. Two unit tests are added. No unsafe code, no cryptographic changes, no parsing/serialization changes, and no denial-of-service vectors are introduced.
Changed components
units/src/amount/ops.rsunits/src/amount/mod.rsInspect captured patch +56 / −2
### units/src/amount/mod.rs
@@ -5,7 +5,7 @@
//! This module mainly introduces the [`Amount`] and [`SignedAmount`] types.
//! We refer to the documentation on the types for more information.
-mod result;
+mod ops;
mod signed;
#[cfg(test)]
mod tests;
### units/src/amount/ops.rs
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: CC0-1.0
-//! Provides a monadic type returned by mathematical operations ([`core::ops`]).
+//! Implements mathematical operators from [`core::ops`] and basic conversions.
use core::num::{NonZeroI64, NonZeroU64};
use core::ops;
@@ -113,11 +113,21 @@ crate::internal_macros::impl_op_for_references! {
fn rem(self, modulus: u64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) }
}
+ impl ops::Rem<NonZeroU64> for Amount {
+ type Output = Amount;
+
+ fn rem(self, modulus: NonZeroU64) -> Self::Output { Self::from_sat(self.to_sat() % modulus.get()).expect("construction from remainder cannot fail") }
+ }
impl ops::Rem<u64> for NumOpResult<Amount> {
type Output = NumOpResult<Amount>;
fn rem(self, modulus: u64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
}
+ impl ops::Rem<NonZeroU64> for NumOpResult<Amount> {
+ type Output = NumOpResult<Amount>;
+
+ fn rem(self, modulus: NonZeroU64) -> Self::Output { self.map(|lhs| lhs % modulus) }
+ }
impl ops::Add<SignedAmount> for SignedAmount {
type Output = NumOpResult<SignedAmount>;
@@ -199,11 +209,21 @@ crate::internal_macros::impl_op_for_references! {
fn rem(self, modulus: i64) -> Self::Output { self.checked_rem(modulus).valid_or_error(MathOp::Rem) }
}
+ impl ops::Rem<NonZeroI64> for SignedAmount {
+ type Output = SignedAmount;
+
+ fn rem(self, modulus: NonZeroI64) -> Self::Output { SignedAmount::from_sat(self.to_sat() % modulus.get()).expect("construction from reamainder cannot fail") }
+ }
impl ops::Rem<i64> for NumOpResult<SignedAmount> {
type Output = NumOpResult<SignedAmount>;
fn rem(self, modulus: i64) -> Self::Output { self.and_then(|lhs| lhs % modulus) }
}
+ impl ops::Rem<NonZeroI64> for NumOpResult<SignedAmount> {
+ type Output = NumOpResult<SignedAmount>;
+
+ fn rem(self, modulus: NonZeroI64) -> Self::Output { self.map(|lhs| lhs % modulus) }
+ }
}
impl_mul_assign!(NumOpResult<Amount>, u64);
@@ -216,6 +236,10 @@ impl_div_assign!(NumOpResult<Amount>, NonZeroU64);
impl_div_assign!(NumOpResult<SignedAmount>, NonZeroI64);
impl_rem_assign!(NumOpResult<Amount>, u64);
impl_rem_assign!(NumOpResult<SignedAmount>, i64);
+impl_rem_assign!(Amount, NonZeroU64);
+impl_rem_assign!(SignedAmount, NonZeroI64);
+impl_rem_assign!(NumOpResult<Amount>, NonZeroU64);
+impl_rem_assign!(NumOpResult<SignedAmount>, NonZeroI64);
impl_add_assign_for_results!(Amount);
impl_add_assign_for_results!(SignedAmount);
@@ -453,6 +477,21 @@ mod tests {
assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
}
+ #[test]
+ fn test_rem_assign_nz_amount() {
+ fn nz(x: u64) -> NonZeroU64 { NonZeroU64::new(x).unwrap() }
+
+ let mut res = Amount::from_sat_u32(100);
+ res %= nz(30_u64);
+ assert_eq!(res, Amount::from_sat_u32(10));
+
+ res %= &nz(4_u64);
+ assert_eq!(res, Amount::from_sat_u32(2));
+
+ res %= nz(5_u64);
+ assert_eq!(res, Amount::from_sat_u32(2));
+ }
+
#[test]
fn test_rem_assign_signed_amount() {
let ssat = SignedAmount::from_sat_i32(-50);
@@ -471,6 +510,21 @@ mod tests {
assert_eq!(res, NumOpResult::Error(NumOpError::while_doing(MathOp::Rem)));
}
+ #[test]
+ fn test_rem_assign_nz_signed_amount() {
+ fn nz(x: i64) -> NonZeroI64 { NonZeroI64::new(x).unwrap() }
+
+ let mut res = SignedAmount::from_sat_i32(-100);
+ res %= nz(30_i64);
+ assert_eq!(res, SignedAmount::from_sat_i32(-10));
+
+ res %= &nz(4_i64);
+ assert_eq!(res, SignedAmount::from_sat_i32(-2));
+
+ res %= nz(5_i64);
+ assert_eq!(res, SignedAmount::from_sat_i32(-2));
+ }
+
#[test]
fn test_div_assign_amount_nonzero() {
let mut amount = Amount::from_sat_u32(100);Risk score
Our methodology →Why this scored 16/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.