feat(key-wallet,key-wallet-manager): add serde derives for AssetLockFundingType and DerivedAddress#761
feat(key-wallet,key-wallet-manager): add serde derives for AssetLockFundingType and DerivedAddress#761lklimek wants to merge 1 commit into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an optional ChangesSerde serialization support for wallet types
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v0.42-dev #761 +/- ##
=============================================
+ Coverage 72.27% 72.28% +0.01%
=============================================
Files 320 320
Lines 70271 70271
=============================================
+ Hits 50786 50794 +8
+ Misses 19485 19477 -8
|
807bc50 to
56a8440
Compare
|
This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them. |
13e9698 to
4dbe0b5
Compare
…undingType and DerivedAddress
Expose optional serde Serialize/Deserialize on two types so downstream
consumers can drop hand-rolled adapters:
* `key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundingType`
(enum) — derives are gated on the existing `serde` feature.
* `key_wallet_manager::DerivedAddress` (struct) — `key-wallet-manager`
previously had no `serde` feature, so this adds one wired to
`key-wallet/serde` and `dashcore/serde`. The struct's `public_key`
field is `dashcore::PublicKey`, which already implements serde under
the `dashcore/serde` feature, so no custom adapter is needed.
Motivation: dashpay/platform PR #3637 (rs-platform-wallet serde support)
currently works around the missing derives with a custom
`serde_adapters::asset_lock_funding_type` module and a
`#[serde(skip)]` on a `PlatformAddressChangeSet::addresses_derived`
field. Once this lands, both workarounds become deletable.
Round-trip test (`derived_address_serde_json_roundtrip`) locks down the
JSON wire shape for `DerivedAddress` so future regressions are caught.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4dbe0b5 to
954eb92
Compare
What this PR does
Adds optional serde derives behind a
serdefeature gate on two types:key_wallet::wallet::managed_wallet_info::asset_lock_builder::AssetLockFundingType(enum)key_wallet_manager::DerivedAddress(struct)key-walletalready had aserdefeature — just the derive line was added.key-wallet-managerhad noserdefeature, so this PR also adds one: optionalserdedep,[features] serde = ["dep:serde", "key-wallet/serde", "dashcore/serde"].DerivedAddress::public_key: [u8; 33]needs a tiny localwith =adapter (serde_pubkey33) becauseserdeonly ships built-in array impls up to length 32. The adapter writes a flat 33-element byte sequence — same shapeserdewould have produced for[u8; 32]— and the newderived_address_serde_json_roundtriptest locks that in.Why
Downstream consumers in
dashpay/platform(specifically PR #3637 —rs-platform-walletserde support) currently work around these missing derives:serde_adapters::asset_lock_funding_typemodule re-implements derive-equivalent serialization with a stableu8wire-tag. Once this PR lands, the adapter becomes deletable (with one caveat the platform PR will resolve: wire-tag stability for pre-serde on-disk blobs).PlatformAddressChangeSet::addresses_derivedfield is#[serde(skip)]becauseDerivedAddresscan't be serialized through. Once this PR lands, the skip can be removed and the field can round-trip normally.Verification
cargo fmt --all -- --checkcleancargo check -p key-wallet --no-default-featuresand--features serdeboth passcargo check -p key-wallet-manager --no-default-featuresand--features serdeboth passcargo clippy -p key-wallet -p key-wallet-manager --tests --all-features --no-deps -- -D warningscleancargo test -p key-wallet -p key-wallet-manager --features serde— 557 tests pass (includes the new round-trip)Companion PR
dashpay/platform#3637 will pick up these derives once this lands; the platform PR is currently working around their absence.
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
New Features
Tests
Chores