fix: Dashify the units. #250
Conversation
WalkthroughThe pull request rebrands denomination string representations from BTC-family tokens ("BTC", "cBTC", "mBTC", etc.) to DASH-family tokens ("DASH", "cDASH", "mDASH", etc.) in the amount module, updating parsing, display/debug text, ambiguous-form lists, and tests. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
dash/src/amount.rs (2)
2126-2147: Fix expected error for"-42 satoshi BTC"vs"satoshi DASH"In
to_string_with_denomination_from_str_roundtrip, the second assertion still passes"-42 satoshi BTC"but expects anUnknownDenomination("satoshi DASH"). With the current parser, that call will returnUnknownDenomination("satoshi BTC"), causing the test to fail.Either adjust the input to use
DASH(to mirror the previous assertion) or change the expected string. For full DASH-ification, the former seems preferable:- assert_eq!( - SignedAmount::from_str("-42 satoshi BTC"), - Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())), - ); + assert_eq!( + SignedAmount::from_str("-42 satoshi DASH"), + Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())), + );
1960-1977: Update Dash formatting tests to expectDASHsuffix instead ofBTCIn the
check_format_non_negative_show_denom!invocations forDash, the suffix is still" BTC"/" BTC ":check_format_non_negative_show_denom! { Dash, " BTC"; … } check_format_non_negative_show_denom! { Dash, " BTC "; … }But
fmt_satoshi_inusesDenomination::Dash::as_str()(now"DASH"), so these tests will now see"… DASH"and fail.Adjust the suffixes to
" DASH"/" DASH ":- Dash, " BTC"; + Dash, " DASH"; … - Dash, " BTC "; + Dash, " DASH ";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash/src/amount.rs(13 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash/src/amount.rs
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Maintain Minimum Supported Rust Version (MSRV) of 1.89; ensure compatibility with Dash Core versions 0.18.0 - 0.21.0
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Maintain Minimum Supported Rust Version (MSRV) of 1.89; ensure compatibility with Dash Core versions 0.18.0 - 0.21.0
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Applies to key-wallet/**/{account,managed_account}/**/*.rs : Use strongly typed enum-based system for `AccountType` with specific variants (Standard, Identity, Masternode, etc.) to provide compile-time safety for different account purposes
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-04T15:51:59.632Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv/CLAUDE.md:0-0
Timestamp: 2025-12-04T15:51:59.632Z
Learning: Applies to dash-spv/**/*.rs : Use trait-based abstractions for core components like `NetworkManager` and `StorageManager` to enable swappable implementations
Applied to files:
dash/src/amount.rs
🔇 Additional comments (6)
dash/src/amount.rs (6)
693-697: Debug output rebranded to DASH – consistent with new unitsBoth
AmountandSignedAmountnow print"… DASH"in theirDebugimpls, matching the new denomination naming while preserving the numeric formatting. No functional issues here.Also applies to: 1113-1116
1626-1647: Zero-amount parsing tests updated to DASH suffixes – looks correctThe
from_str_zerotest now iterates over["DASH", "mDASH", "uDASH", "nDASH", "pDASH", "bits", "sats", "msats"], which matches the new recognized forms for zero and-0handling. Behavior stays the same, just with DASH units.
1787-1807:to_string_with_denominationexpectations correctly switched to DASHThe updated expectations like
"1 DASH","0.00000001 DASH", and"-0.00000042 DASH"align withDenomination::Dash’sas_str()and the formatting logic. The roundtrip behavior for these cases should remain intact.
2012-2047: From-str tests for DASH denominations are consistentThe
from_str/scasetests you updated ("x DASH","5 DASH DASH","-1 DASH","0.123456 mDASH", etc.) correctly exercise the new DASH forms and error conditions (InvalidCharacter,UnknownDenomination,Negative,TooPrecise,TooBig). The semantics of the checks look unchanged apart from the unit name.
2363-2393: DASH denomination string tests look good; unknown/case-variants are sensibleThe updated
denomination_string_acceptable_formslist now covers the main DASH strings ("DASH","dash","mDASH","mdash","uDASH","udash","nDASH","pDASH") plus the sat/bit variants, matchingDenomination::forms. Likewise, theunknownlist ("NDASH","UDASH","ABC","abc","cBtC","Sat","Sats") appropriately exercises rejected/case-mismatched forms.Once the
CONFUSING_FORMSvs."MDash"mismatch is fixed (see earlier comment), this whole block should be in good shape.
16-32: Doctest uses BTC tokens, but they are still supported—no action requiredThe doctest examples at lines 23–28 do use
"BTC","cBTC", etc., but these tokens are not removed; they remain valid forms. TheDenomination::forms()method explicitly accepts"BTC" | "btc"and maps toDenomination::Dash, along with the other BTC-family variants. The examples will parse correctly and the assertions will pass. No doctest update is needed, and there is no breaking change to external callers.The code maintains backward compatibility with BTC token strings while using
Dash/CentiDashas enum variant names.Likely an incorrect or invalid review comment.
| /// 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"]; | ||
|
|
There was a problem hiding this comment.
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.
pauldelucia
left a comment
pauldelucia
left a comment
There was a problem hiding this comment.
Generally LGTM but I think we should take it even further and replace all Satoshi references with Duff, and probably get rid of remaining BTC references.
|
Yes, I would like that. Sat -> Duff. |
xdustinface
left a comment
xdustinface
left a comment
There was a problem hiding this comment.
@kxcd Looks good in general but CI is failing. It needs cargo formatting run and the test are failing. There are a few more places that need adjustment for this to work.
fix: cargo fmt fix for xk
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
dash/src/amount.rs (4)
3-32: Update module documentation and doc examples to use DASH denomination strings.The module doc at line 3 still says "Bitcoin amounts" and the doc examples at lines 23-28 still use BTC-based denomination strings (
"1 BTC","1 cBTC", etc.). SinceDenomination::formswas updated to only recognize DASH variants, these doc examples will fail to compile/run.Apply this diff:
-//! Bitcoin amounts. +//! Dash amounts. //! //! This module mainly introduces the [Amount] and [SignedAmount] types. //! We refer to the documentation on the types for more information. //! ... -/// assert_eq!(Amount::from_str("1 BTC").unwrap(), Amount::from_sat(100_000_000)); -/// assert_eq!(Amount::from_str("1 cBTC").unwrap(), Amount::from_sat(1_000_000)); -/// assert_eq!(Amount::from_str("1 mBTC").unwrap(), Amount::from_sat(100_000)); -/// assert_eq!(Amount::from_str("1 uBTC").unwrap(), Amount::from_sat(100)); -/// assert_eq!(Amount::from_str("10 nBTC").unwrap(), Amount::from_sat(1)); -/// assert_eq!(Amount::from_str("10000 pBTC").unwrap(), Amount::from_sat(1)); +/// assert_eq!(Amount::from_str("1 DASH").unwrap(), Amount::from_sat(100_000_000)); +/// assert_eq!(Amount::from_str("1 cDASH").unwrap(), Amount::from_sat(1_000_000)); +/// assert_eq!(Amount::from_str("1 mDASH").unwrap(), Amount::from_sat(100_000)); +/// assert_eq!(Amount::from_str("1 uDASH").unwrap(), Amount::from_sat(100)); +/// assert_eq!(Amount::from_str("10 nDASH").unwrap(), Amount::from_sat(1)); +/// assert_eq!(Amount::from_str("10000 pDASH").unwrap(), Amount::from_sat(1));
120-125: Update doc comment to reflect DASH denominations.The comment at lines 123-124 still references
"BTC, mBTC, uBTC, nBTC, pBTC"but the implementation now parses DASH variants./// Any combination of upper and/or lower case, excluding uppercase of SI(m, u, n, p) is considered valid. - /// - Singular: BTC, mBTC, uBTC, nBTC, pBTC + /// - Singular: DASH, mDASH, uDASH, nDASH, pDASH /// - Plural or singular: sat, satoshi, bit, msat
479-483: Update struct documentation to reference Dash.The comment still says "Bitcoin amounts" but this is a Dash library.
-/// The [Amount] type can be used to express Bitcoin amounts that support +/// The [Amount] type can be used to express Dash amounts that support
1960-1977: Fix incorrect denomination suffix in test expectations.These tests use
" BTC"as the expected denomination suffix, butDenomination::Dash.as_str()now returns"DASH". The tests will fail because the actual output will be"0.00000001 DASH"but the assertion expects"0.00000001 BTC".check_format_non_negative_show_denom! { - Dash, " BTC"; + Dash, " DASH"; btc_check_fmt_non_negative_show_denom_0, 1, "{:14.1}", "0.00000001"; ... } check_format_non_negative_show_denom! { - Dash, " BTC "; + Dash, " DASH "; btc_check_fmt_non_negative_show_denom_align_0, 1, "{:<15}", "0.00000001"; ... }
♻️ Duplicate comments (2)
dash/src/amount.rs (2)
106-109: Fix mismatch betweenCONFUSING_FORMSand test expectations.The constant contains
"Mdash"but the test at line 2381 expects"MDash". This will causeDenomination::from_str("MDash")to returnUnknownDenominationrather thanPossiblyConfusingDenomination, failing the test.
2379-2388: Test expects "MDash" butCONFUSING_FORMScontains "Mdash".This test will fail because
"MDash"is not inCONFUSING_FORMS(which has"Mdash"), soDenomination::from_str("MDash")will returnUnknownDenominationinstead ofPossiblyConfusingDenomination.Either update
CONFUSING_FORMSat line 109 to include"MDash", or update this test to use"Mdash".
🧹 Nitpick comments (1)
dash/src/amount.rs (1)
529-532: Consider updating remaining Bitcoin/BTC references in documentation.Multiple doc comments throughout the file still reference "Bitcoin" or "BTC" (e.g., lines 529, 565, 607, 700, 799, 860, 905, 943, 985, 1120). For consistency with the DASH rebranding, consider updating these to reference "Dash" and "DASH".
Also applies to: 565-567, 605-608, 699-701, 793-800, 858-862, 905-908, 941-948, 983-986, 1119-1121
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
dash/src/amount.rs(13 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Never hardcode network parameters, addresses, or keys in Rust code
Use proper error types with thiserror crate and propagate errors appropriately in Rust code
Use tokio runtime for async operations in Rust code
Use conditional compilation with feature flags for optional features
Write unit tests for new functionality in Rust
Format Rust code with cargo fmt before committing
Ensure clippy passes with all warnings as errors on all Rust code
**/*.rs: Each crate keeps sources insrc/; unit tests live alongside code with#[cfg(test)]
Maintain MSRV of 1.89 for Rust code
Format Rust code withrustfmt(seerustfmt.toml); runcargo fmt --allbefore commits
Lint Rust code withclippy; avoidunwrap()/expect()in library code; use error types (e.g.,thiserror)
Usesnake_casefor function and variable names in Rust
UseUpperCamelCasefor types and traits in Rust
UseSCREAMING_SNAKE_CASEfor constants in Rust
Follow mixed editions (2021/2024) and crate-specific idioms; prefer async viatokiowhere applicable
Unit tests should be placed near code with descriptive names (e.g.,test_parse_address_mainnet)
Use#[ignore]for network-dependent or long-running tests; run with-- --ignoredflag
Never commit secrets or real keys; avoid logging sensitive data in Rust code
Keep test vectors deterministic in Rust test code
Files:
dash/src/amount.rs
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
📚 Learning: 2025-12-01T08:00:50.618Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:50.618Z
Learning: Applies to swift-dash-core-sdk/**/*.rs : Implement new FFI functions in Rust with `#[no_mangle] extern "C"` annotation in dash-spv-ffi
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Keep test vectors updated with Dash Core, monitor for new DIPs affecting wallet structure, maintain backward compatibility for serialized wallets, and update derivation paths for Platform features
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Rust strings must be returned as `*const c_char` with caller responsibility to free using `dash_string_free`
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:00:37.919Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:00:37.919Z
Learning: Maintain Minimum Supported Rust Version (MSRV) of 1.89; ensure compatibility with Dash Core versions 0.18.0 - 0.21.0
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:01:18.174Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T08:01:18.174Z
Learning: Organize workspace with crates: `dash`, `hashes`, `internals`, `dash-network`, `dash-spv`, `key-wallet`, `rpc-*`, utilities (`fuzz`, `test-utils`), and FFI crates (`*-ffi`)
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Input strings in FFI functions are `*const c_char` (borrowed, not freed by C caller)
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T07:59:58.608Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: dash-spv-ffi/CLAUDE.md:0-0
Timestamp: 2025-12-01T07:59:58.608Z
Learning: Applies to dash-spv-ffi/src/**/*.rs : Add cbindgen annotations for complex types in FFI functions
Applied to files:
dash/src/amount.rs
📚 Learning: 2025-12-01T08:01:00.652Z
Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: swift-dash-core-sdk/Examples/DashHDWalletExample/CLAUDE.md:0-0
Timestamp: 2025-12-01T08:01:00.652Z
Learning: Applies to swift-dash-core-sdk/Examples/DashHDWalletExample/**/*.swift : Add public wrapper methods to DashSDK if needed functionality is not exposed by public API
Applied to files:
dash/src/amount.rs
🔇 Additional comments (8)
dash/src/amount.rs (8)
75-84: LGTM!The denomination string representations are correctly updated to DASH variants while maintaining the SI prefix conventions.
88-103: LGTM!The denomination parsing forms are correctly updated to recognize DASH variants with consistent uppercase/lowercase handling.
693-696: LGTM!Debug output correctly updated to display "DASH".
1627-1648: LGTM!Test correctly updated to use DASH denomination strings.
1797-1807: LGTM!Test expectations correctly updated to expect "DASH" in formatted output.
2012-2050: LGTM!Error case tests correctly updated for DASH denomination parsing.
2369-2376: LGTM!Valid denomination forms list correctly updated to include DASH variants.
2391-2402: LGTM!Unknown denomination test correctly includes uppercase prefix variants like "NDASH" and "UDASH" which should be rejected since only lowercase prefixes are valid.
| assert_eq!( | ||
| SignedAmount::from_str("-42 satoshi BTC"), | ||
| Err(ParseAmountError::UnknownDenomination("satoshi BTC".into())), | ||
| Err(ParseAmountError::UnknownDenomination("satoshi DASH".into())), | ||
| ); |
There was a problem hiding this comment.
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.
| 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.
4 participants
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.