chore(key-wallet): drop no-std supp for key-wallet#518
Conversation
📝 WalkthroughWalkthroughThis PR removes conditional Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #518 +/- ##
=============================================
- Coverage 67.44% 67.25% -0.20%
=============================================
Files 318 318
Lines 67020 67025 +5
=============================================
- Hits 45204 45079 -125
- Misses 21816 21946 +130
|
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
18273b0 to
b2ff339
Compare
b2ff339 to
eb5b21b
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
eb5b21b to
2f9145a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@key-wallet/Cargo.toml`:
- Around line 11-12: The PR title prefix is too weak for the change that removes
a previously supported build feature in the Cargo.toml default features list
(the default = ["secp256k1/std", "bip39/std", "getrandom", "rand"] line); update
the PR title to a user-visible prefix such as feat: or breaking: (and adjust the
PR description) to reflect that a public build-mode/feature was dropped, and
ensure any release notes or changelog mention the changed default features so
consumers are aware of the compatibility impact.
- Around line 11-12: The crate removed the public feature alias "std" causing
downstream breakage; restore a named feature "std" that expands to the component
features (e.g., "dashcore_hashes/std", "secp256k1/std", "bip39/std",
"getrandom", "rand") and then make default = ["std"] so the default still
enables those components while preserving compatibility for manifests that
explicitly request the "std" feature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 521ce552-357b-4ce4-8098-726376c2bf7e
📒 Files selected for processing (48)
key-wallet-ffi/Cargo.tomlkey-wallet/Cargo.tomlkey-wallet/src/account/account_collection.rskey-wallet/src/account/account_trait.rskey-wallet/src/account/bls_account.rskey-wallet/src/account/eddsa_account.rskey-wallet/src/bip32.rskey-wallet/src/bip38.rskey-wallet/src/derivation.rskey-wallet/src/derivation_bls_bip32.rskey-wallet/src/derivation_slip10.rskey-wallet/src/error.rskey-wallet/src/lib.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/managed_account/managed_platform_account.rskey-wallet/src/managed_account/metadata.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/transaction_record.rskey-wallet/src/manager/events.rskey-wallet/src/manager/matching.rskey-wallet/src/manager/mod.rskey-wallet/src/manager/process_block.rskey-wallet/src/manager/wallet_interface.rskey-wallet/src/mnemonic.rskey-wallet/src/psbt/error.rskey-wallet/src/psbt/map/global.rskey-wallet/src/psbt/map/input.rskey-wallet/src/psbt/map/mod.rskey-wallet/src/psbt/map/output.rskey-wallet/src/psbt/mod.rskey-wallet/src/psbt/raw.rskey-wallet/src/psbt/serialize.rskey-wallet/src/seed.rskey-wallet/src/tests/wallet_tests.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/wallet/bip38.rskey-wallet/src/wallet/helper.rskey-wallet/src/wallet/initialization.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/metadata.rskey-wallet/src/wallet/mod.rskey-wallet/src/wallet/root_extended_keys.rs
💤 Files with no reviewable changes (28)
- key-wallet/src/account/bls_account.rs
- key-wallet/src/account/account_trait.rs
- key-wallet/src/wallet/helper.rs
- key-wallet/src/manager/matching.rs
- key-wallet/src/managed_account/address_pool.rs
- key-wallet/src/wallet/bip38.rs
- key-wallet/src/bip38.rs
- key-wallet/src/derivation.rs
- key-wallet/src/wallet/mod.rs
- key-wallet/src/manager/events.rs
- key-wallet/src/wallet/initialization.rs
- key-wallet/src/wallet/managed_wallet_info/helpers.rs
- key-wallet/src/lib.rs
- key-wallet/src/tests/wallet_tests.rs
- key-wallet/src/psbt/serialize.rs
- key-wallet/src/account/eddsa_account.rs
- key-wallet/src/managed_account/transaction_record.rs
- key-wallet/src/psbt/raw.rs
- key-wallet/src/psbt/map/mod.rs
- key-wallet/src/managed_account/metadata.rs
- key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
- key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
- key-wallet/src/seed.rs
- key-wallet/src/psbt/error.rs
- key-wallet/src/error.rs
- key-wallet/src/bip32.rs
- key-wallet/src/mnemonic.rs
- key-wallet/src/wallet/managed_wallet_info/mod.rs
✅ Files skipped from review due to trivial changes (12)
- key-wallet/src/managed_account/managed_account_collection.rs
- key-wallet/src/psbt/map/global.rs
- key-wallet/src/transaction_checking/account_checker.rs
- key-wallet/src/account/account_collection.rs
- key-wallet/src/manager/process_block.rs
- key-wallet/src/psbt/map/input.rs
- key-wallet/src/managed_account/managed_account_trait.rs
- key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
- key-wallet-ffi/Cargo.toml
- key-wallet/src/psbt/map/output.rs
- key-wallet/src/wallet/root_extended_keys.rs
- key-wallet/src/wallet/metadata.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- key-wallet/src/managed_account/managed_platform_account.rs
- key-wallet/src/managed_account/mod.rs
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
2f9145a to
9e10a8f
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
key-wallet/src/psbt/mod.rs (1)
576-577: Add smoke tests for the unconditionalHashMap/HashSetGetKeyimpls.This PR makes those impls part of the always-on API surface, but this file still only exercises
BTreeMap. A small pair of tests would catch accidental cfg regressions or trait-bound breakage.As per coding guidelines, "Write unit tests for new functionality in Rust code".
Also applies to: 596-597
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@key-wallet/src/psbt/mod.rs` around lines 576 - 577, Add two small unit tests that exercise the unconditional GetKey implementations generated by impl_get_key_for_set! (and the corresponding HashMap impl at the other site) to prevent cfg/trait regressions: create a test that constructs a HashSet, inserts a value and asserts that GetKey::get_key (or the public helper used in this module) returns the expected key, and a second test that does the same for a HashMap (insert a (key,value) pair and assert get_key returns the stored key or lookup behaves as expected). Place these tests in the same mod.rs test module alongside the existing BTreeMap tests so they run under normal builds and validate the always-on API surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@key-wallet/src/psbt/mod.rs`:
- Around line 576-577: Add two small unit tests that exercise the unconditional
GetKey implementations generated by impl_get_key_for_set! (and the corresponding
HashMap impl at the other site) to prevent cfg/trait regressions: create a test
that constructs a HashSet, inserts a value and asserts that GetKey::get_key (or
the public helper used in this module) returns the expected key, and a second
test that does the same for a HashMap (insert a (key,value) pair and assert
get_key returns the stored key or lookup behaves as expected). Place these tests
in the same mod.rs test module alongside the existing BTreeMap tests so they run
under normal builds and validate the always-on API surface.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 15b4c72f-90a9-412d-a9e9-4ad34924a7a8
📒 Files selected for processing (48)
key-wallet-ffi/Cargo.tomlkey-wallet/Cargo.tomlkey-wallet/src/account/account_collection.rskey-wallet/src/account/account_trait.rskey-wallet/src/account/bls_account.rskey-wallet/src/account/eddsa_account.rskey-wallet/src/bip32.rskey-wallet/src/bip38.rskey-wallet/src/derivation.rskey-wallet/src/derivation_bls_bip32.rskey-wallet/src/derivation_slip10.rskey-wallet/src/error.rskey-wallet/src/lib.rskey-wallet/src/managed_account/address_pool.rskey-wallet/src/managed_account/managed_account_collection.rskey-wallet/src/managed_account/managed_account_trait.rskey-wallet/src/managed_account/managed_platform_account.rskey-wallet/src/managed_account/metadata.rskey-wallet/src/managed_account/mod.rskey-wallet/src/managed_account/transaction_record.rskey-wallet/src/manager/events.rskey-wallet/src/manager/matching.rskey-wallet/src/manager/mod.rskey-wallet/src/manager/process_block.rskey-wallet/src/manager/wallet_interface.rskey-wallet/src/mnemonic.rskey-wallet/src/psbt/error.rskey-wallet/src/psbt/map/global.rskey-wallet/src/psbt/map/input.rskey-wallet/src/psbt/map/mod.rskey-wallet/src/psbt/map/output.rskey-wallet/src/psbt/mod.rskey-wallet/src/psbt/raw.rskey-wallet/src/psbt/serialize.rskey-wallet/src/seed.rskey-wallet/src/tests/wallet_tests.rskey-wallet/src/transaction_checking/account_checker.rskey-wallet/src/wallet/bip38.rskey-wallet/src/wallet/helper.rskey-wallet/src/wallet/initialization.rskey-wallet/src/wallet/managed_wallet_info/coin_selection.rskey-wallet/src/wallet/managed_wallet_info/helpers.rskey-wallet/src/wallet/managed_wallet_info/mod.rskey-wallet/src/wallet/managed_wallet_info/transaction_builder.rskey-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rskey-wallet/src/wallet/metadata.rskey-wallet/src/wallet/mod.rskey-wallet/src/wallet/root_extended_keys.rs
💤 Files with no reviewable changes (28)
- key-wallet/src/account/account_trait.rs
- key-wallet/src/managed_account/transaction_record.rs
- key-wallet/src/wallet/bip38.rs
- key-wallet/src/wallet/helper.rs
- key-wallet/src/psbt/serialize.rs
- key-wallet/src/wallet/managed_wallet_info/helpers.rs
- key-wallet/src/wallet/initialization.rs
- key-wallet/src/account/eddsa_account.rs
- key-wallet/src/psbt/raw.rs
- key-wallet/src/bip38.rs
- key-wallet/src/wallet/managed_wallet_info/mod.rs
- key-wallet/src/lib.rs
- key-wallet/src/managed_account/address_pool.rs
- key-wallet/src/manager/matching.rs
- key-wallet/src/psbt/map/mod.rs
- key-wallet/src/wallet/mod.rs
- key-wallet/src/derivation.rs
- key-wallet/src/managed_account/metadata.rs
- key-wallet/src/manager/events.rs
- key-wallet/src/tests/wallet_tests.rs
- key-wallet/src/seed.rs
- key-wallet/src/wallet/managed_wallet_info/coin_selection.rs
- key-wallet/src/error.rs
- key-wallet/src/psbt/error.rs
- key-wallet/src/account/bls_account.rs
- key-wallet/src/bip32.rs
- key-wallet/src/mnemonic.rs
- key-wallet/src/wallet/managed_wallet_info/transaction_builder.rs
✅ Files skipped from review due to trivial changes (11)
- key-wallet/src/managed_account/managed_account_trait.rs
- key-wallet/src/wallet/root_extended_keys.rs
- key-wallet/src/transaction_checking/account_checker.rs
- key-wallet/src/psbt/map/global.rs
- key-wallet/src/managed_account/managed_account_collection.rs
- key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs
- key-wallet/src/psbt/map/input.rs
- key-wallet/src/psbt/map/output.rs
- key-wallet/src/wallet/metadata.rs
- key-wallet/src/manager/process_block.rs
- key-wallet/src/account/account_collection.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- key-wallet-ffi/Cargo.toml
- key-wallet/src/managed_account/mod.rs
- key-wallet/src/manager/wallet_interface.rs
- key-wallet/src/derivation_bls_bip32.rs
- key-wallet/src/managed_account/managed_platform_account.rs
- key-wallet/Cargo.toml
Summary by CodeRabbit
New Features
Chores