Skip to content

fix: Dashify the units. #250

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kxcd wants to merge 3 commits into
base: v0.41-dev
Choose a base branch
from
Open

fix: Dashify the units. #250

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5e8907
Dashify the units.
kxcd Dec 7, 2025
8130cd1
fmt
QuantumExplorer Dec 9, 2025
e884215
Merge pull request #1 from dashpay/fix/cargofmtforxk
kxcd Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 42 additions & 38 deletions dash/src/amount.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,12 @@ impl Denomination {
/// Returns stringly representation of this
fn as_str(self) -> &'static str {
match self {
Denomination::Dash => "BTC",
Denomination::CentiDash => "cBTC",
Denomination::MilliDash => "mBTC",
Denomination::MicroDash => "uBTC",
Denomination::NanoDash => "nBTC",
Denomination::PicoDash => "pBTC",
Denomination::Dash => "DASH",
Denomination::CentiDash => "cDASH",
Denomination::MilliDash => "mDASH",
Denomination::MicroDash => "uDASH",
Denomination::NanoDash => "nDASH",
Denomination::PicoDash => "pDASH",
Denomination::Bit => "bits",
Denomination::Satoshi => "satoshi",
Denomination::MilliSatoshi => "msat",
Expand All @@ -87,12 +87,12 @@ impl Denomination {
/// The different str forms of denominations that are recognized.
fn forms(s: &str) -> Option<Self> {
match s {
"BTC" | "btc" => Some(Denomination::Dash),
"cBTC" | "cbtc" => Some(Denomination::CentiDash),
"mBTC" | "mbtc" => Some(Denomination::MilliDash),
"uBTC" | "ubtc" => Some(Denomination::MicroDash),
"nBTC" | "nbtc" => Some(Denomination::NanoDash),
"pBTC" | "pbtc" => Some(Denomination::PicoDash),
"DASH" | "dash" => Some(Denomination::Dash),
"cDASH" | "cdash" => Some(Denomination::CentiDash),
"mDASH" | "mdash" => Some(Denomination::MilliDash),
"uDASH" | "udash" => Some(Denomination::MicroDash),
"nDASH" | "ndash" => Some(Denomination::NanoDash),
"pDASH" | "pdash" => Some(Denomination::PicoDash),
"bit" | "bits" | "BIT" | "BITS" => Some(Denomination::Bit),
"SATOSHI" | "satoshi" | "SATOSHIS" | "satoshis" | "SAT" | "sat" | "SATS" | "sats" => {
Some(Denomination::Satoshi)
Expand All @@ -106,7 +106,7 @@ impl Denomination {
/// These form are ambiguous and could have many meanings. For example, M could denote Mega or Milli.
/// If any of these forms are used, an error type PossiblyConfusingDenomination is returned.
const CONFUSING_FORMS: [&str; 9] =
["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MBTC", "Mbtc", "PBTC"];
["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MDASH", "Mdash", "PDASH"];

Comment on lines 106 to 110
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix mismatch between CONFUSING_FORMS and disallow_confusing_forms for MDash

CONFUSING_FORMS contains "MDASH" and "Mdash", but the disallow_confusing_forms test checks for "MDASH" and "MDash". As written, "MDash" is not in CONFUSING_FORMS, so Denomination::from_str("MDash") will return UnknownDenomination rather than PossiblyConfusingDenomination, and the test will fail.

Align them by updating the constant (and, if desired, covering both variants):

-const CONFUSING_FORMS: [&str; 9] =
-    ["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MDASH", "Mdash", "PDASH"];
+const CONFUSING_FORMS: [&str; 9] =
+    ["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MDASH", "MDash", "PDASH"];

(The valid and unknown lists for denomination strings otherwise look good and match the new DASH-oriented behavior.)

Also applies to: 2363-2393

🤖 Prompt for AI Agents 
In dash/src/amount.rs around lines 106 to 110 (and similarly at lines
~2363-2393), the CONFUSING_FORMS constant contains "MDASH" and "Mdash" but the
test disallow_confusing_forms expects "MDash"; add the missing variant "MDash"
to CONFUSING_FORMS (or replace "Mdash" with "MDash", or include both "Mdash" and
"MDash") so the constant and the test match and Denomination::from_str("MDash")
yields PossiblyConfusingDenomination.

impl fmt::Display for Denomination {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
Expand Down Expand Up @@ -692,7 +692,7 @@ impl default::Default for Amount {

impl fmt::Debug for Amount {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Amount({:.8} BTC)", self.to_dash())
write!(f, "Amount({:.8} DASH)", self.to_dash())
}
}

Expand Down Expand Up @@ -1112,7 +1112,7 @@ impl default::Default for SignedAmount {

impl fmt::Debug for SignedAmount {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "SignedAmount({:.8} BTC)", self.to_btc())
write!(f, "SignedAmount({:.8} DASH)", self.to_btc())
}
}

Expand Down Expand Up @@ -1625,7 +1625,7 @@ mod tests {

#[test]
fn from_str_zero() {
let denoms = vec!["BTC", "mBTC", "uBTC", "nBTC", "pBTC", "bits", "sats", "msats"];
let denoms = vec!["DASH", "mDASH", "uDASH", "nDASH", "pDASH", "bits", "sats", "msats"];
for denom in denoms {
for v in &["0", "000"] {
let s = format!("{} {}", v, denom);
Expand Down Expand Up @@ -1794,16 +1794,16 @@ mod tests {
assert_eq!(Amount::ONE_SAT.to_string_in(D::Dash), "0.00000001");
assert_eq!(SignedAmount::from_sat(-42).to_string_in(D::Dash), "-0.00000042");

assert_eq!(Amount::ONE_BTC.to_string_with_denomination(D::Dash), "1 BTC");
assert_eq!(Amount::ONE_BTC.to_string_with_denomination(D::Dash), "1 DASH");
assert_eq!(Amount::ONE_SAT.to_string_with_denomination(D::MilliSatoshi), "1000 msat");
assert_eq!(
SignedAmount::ONE_BTC.to_string_with_denomination(D::Satoshi),
"100000000 satoshi"
);
assert_eq!(Amount::ONE_SAT.to_string_with_denomination(D::Dash), "0.00000001 BTC");
assert_eq!(Amount::ONE_SAT.to_string_with_denomination(D::Dash), "0.00000001 DASH");
assert_eq!(
SignedAmount::from_sat(-42).to_string_with_denomination(D::Dash),
"-0.00000042 BTC"
"-0.00000042 DASH"
);
}

Expand Down Expand Up @@ -2009,11 +2009,14 @@ mod tests {
fn from_str() {
use super::ParseAmountError as E;

assert_eq!(Amount::from_str("x BTC"), Err(E::InvalidCharacter('x')));
assert_eq!(Amount::from_str("xBTC"), Err(E::UnknownDenomination("xBTC".into())));
assert_eq!(Amount::from_str("5 BTC BTC"), Err(E::UnknownDenomination("BTC BTC".into())));
assert_eq!(Amount::from_str("5BTC BTC"), Err(E::InvalidCharacter('B')));
assert_eq!(Amount::from_str("5 5 BTC"), Err(E::UnknownDenomination("5 BTC".into())));
assert_eq!(Amount::from_str("x DASH"), Err(E::InvalidCharacter('x')));
assert_eq!(Amount::from_str("xDASH"), Err(E::UnknownDenomination("xDASH".into())));
assert_eq!(
Amount::from_str("5 DASH DASH"),
Err(E::UnknownDenomination("DASH DASH".into()))
);
assert_eq!(Amount::from_str("5DASH DASH"), Err(E::InvalidCharacter('D')));
assert_eq!(Amount::from_str("5 5 DASH"), Err(E::UnknownDenomination("5 DASH".into())));

#[track_caller]
fn case(s: &str, expected: Result<Amount, ParseAmountError>) {
Expand All @@ -2029,20 +2032,20 @@ mod tests {

case("5 BCH", Err(E::UnknownDenomination("BCH".to_owned())));

case("-1 BTC", Err(E::Negative));
case("-0.0 BTC", Err(E::Negative));
case("0.123456789 BTC", Err(E::TooPrecise));
case("-1 DASH", Err(E::Negative));
case("-0.0 DASH", Err(E::Negative));
case("0.123456789 DASH", Err(E::TooPrecise));
scase("-0.1 satoshi", Err(E::TooPrecise));
case("0.123456 mBTC", Err(E::TooPrecise));
case("0.123456 mDASH", Err(E::TooPrecise));
scase("-1.001 bits", Err(E::TooPrecise));
scase("-200000000000 BTC", Err(E::TooBig));
scase("-200000000000 DASH", Err(E::TooBig));
case("18446744073709551616 sat", Err(E::TooBig));

case(".5 bits", Ok(Amount::from_sat(50)));
scase("-.5 bits", Ok(SignedAmount::from_sat(-50)));
case("0.00253583 BTC", Ok(Amount::from_sat(253583)));
case("0.00253583 DASH", Ok(Amount::from_sat(253583)));
scase("-5 satoshi", Ok(SignedAmount::from_sat(-5)));
case("0.10000000 BTC", Ok(Amount::from_sat(100_000_00)));
case("0.10000000 DASH", Ok(Amount::from_sat(100_000_00)));
scase("-100 bits", Ok(SignedAmount::from_sat(-10_000)));
}

Expand Down Expand Up @@ -2138,12 +2141,12 @@ mod tests {
assert_eq!(Amount::from_str(&denom(amt, D::PicoDash)), Ok(amt));

assert_eq!(
Amount::from_str("42 satoshi BTC"),
Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())),
Amount::from_str("42 satoshi DASH"),
Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())),
);
assert_eq!(
SignedAmount::from_str("-42 satoshi BTC"),
Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())),
Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())),
);
Comment on lines 2147 to 2150
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix incorrect test expectation.

The test parses "-42 satoshi BTC" but expects the error to contain "satoshi DASH". The error should reflect the actual parsed input: "satoshi BTC".

         assert_eq!(
             SignedAmount::from_str("-42 satoshi BTC"),
-            Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())),
+            Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())),
         );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assert_eq!(
SignedAmount::from_str("-42 satoshi BTC"),
Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())),
Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())),
);
assert_eq!(
SignedAmount::from_str("-42 satoshi BTC"),
Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())),
);
🤖 Prompt for AI Agents 
In dash/src/amount.rs around lines 2147 to 2150, the unit test expects the wrong
denomination in the ParseAmountError; it parses "-42 satoshi BTC" but asserts
the error contains "satoshi DASH". Update the test expectation to assert
Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())) so the error
matches the actual input string.

}

Expand Down Expand Up @@ -2364,8 +2367,8 @@ mod tests {
fn denomination_string_acceptable_forms() {
// Non-exhaustive list of valid forms.
let valid = vec![
"BTC", "btc", "mBTC", "mbtc", "uBTC", "ubtc", "SATOSHI", "satoshi", "SATOSHIS",
"satoshis", "SAT", "sat", "SATS", "sats", "bit", "bits", "nBTC", "pBTC",
"DASH", "dash", "mDASH", "mdash", "uDASH", "udash", "SATOSHI", "satoshi", "SATOSHIS",
"satoshis", "SAT", "sat", "SATS", "sats", "bit", "bits", "nDASH", "pDASH",
];
for denom in valid.iter() {
assert!(Denomination::from_str(denom).is_ok());
Expand All @@ -2374,7 +2377,8 @@ mod tests {

#[test]
fn disallow_confusing_forms() {
let confusing = ["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MBTC", "Mbtc", "PBTC"];
let confusing =
["Msat", "Msats", "MSAT", "MSATS", "MSat", "MSats", "MDASH", "MDash", "PDASH"];
for denom in confusing.iter() {
match Denomination::from_str(denom) {
Ok(_) => panic!("from_str should error for {}", denom),
Expand All @@ -2387,7 +2391,7 @@ mod tests {
#[test]
fn disallow_unknown_denomination() {
// Non-exhaustive list of unknown forms.
let unknown = ["NBTC", "UBTC", "ABC", "abc", "cBtC", "Sat", "Sats"];
let unknown = ["NDASH", "UDASH", "ABC", "abc", "cBtC", "Sat", "Sats"];
for denom in unknown.iter() {
match Denomination::from_str(denom) {
Ok(_) => panic!("from_str should error for {}", denom),
Expand Down
Loading