feat(wasm-dpp2): add platform address transition WASM bindings#3147
Open
feat(wasm-dpp2): add platform address transition WASM bindings#3147
Conversation
Contributor
📝 WalkthroughWalkthroughThe PR adds WebAssembly bindings for six platform address transition types (AddressCreditWithdrawal, AddressFundingFromAssetLock, AddressFundsTransfer, IdentityCreateFromAddresses, IdentityCreditTransferToAddresses, IdentityTopUpFromAddresses) with full construction, serialization, deserialization, and field mutation APIs. Helper functions extract and validate inputs/outputs from JavaScript options objects, and comprehensive unit tests validate all functionality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/wasm-dpp2/src/platform_address/transitions/identity_credit_transfer_to_addresses_transition.rs (1)
159-175: Sameoutputs_to_btree_mappanic risk applies here.
set_recipient_addresses(Line 174) and the constructor (Line 96) both useoutputs_to_btree_map, which internally callsinto_inner()with an.expect(). If aPlatformAddressOutputwithout an amount reaches this path, it will panic. This is the same root cause flagged in theaddress_credit_withdrawal_transition.rsreview.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/src/platform_address/transitions/identity_credit_transfer_to_addresses_transition.rs` around lines 159 - 175, The setter set_recipient_addresses calls outputs_to_btree_map which currently uses into_inner().expect() and can panic if a PlatformAddressOutput lacks an amount; change outputs_to_btree_map to return a Result<BTreeMap<...>, JsValue> (or custom error) instead of unwrapping, update set_recipient_addresses (and the constructor that also calls outputs_to_btree_map) to handle the Result by propagating the error to JS (e.g., return Result<(), JsValue> or use wasm_bindgen::throw_str) and validate each PlatformAddressOutputWasm for a present amount before converting so no .expect() is used. Ensure references are updated for outputs_to_btree_map, set_recipient_addresses, and the constructor that calls outputs_to_btree_map.
🧹 Nitpick comments (9)
packages/wasm-dpp2/tests/unit/AddressFundsTransfer.spec.ts (1)
42-45: Filename uses PascalCase instead of kebab-case.Per coding guidelines, files matching
packages/**/!(node_modules)/**/*.{js,jsx,ts,tsx}should use kebab-case for filenames. This file isAddressFundsTransfer.spec.tsbut should beaddress-funds-transfer.spec.ts. The same applies to the other new test files in this PR. However, if PascalCase is the established convention inwasm-dpp2tests, feel free to disregard.As per coding guidelines: "Use kebab-case for filenames within JS packages"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/tests/unit/AddressFundsTransfer.spec.ts` around lines 42 - 45, The test filename uses PascalCase (AddressFundsTransfer.spec.ts) but should follow the repo's kebab-case rule; rename the file to address-funds-transfer.spec.ts and update any imports or test references that point to AddressFundsTransfer.spec.ts (and do the same for other new test files in this PR) so test runner and imports resolve correctly; verify references in test suites or any index files that mention AddressFundsTransferTransition or the skipped describe('AddressFundsTransferTransition (.build API)') remain valid after the rename.packages/wasm-dpp2/src/platform_address/input_output.rs (1)
186-265: Consider extracting a generic helper to reduce duplication betweeninputs_from_js_optionsandoutputs_from_js_options.Both functions share identical scaffolding (Reflect::get → null check → array check → enumerate →
to_wasm→ clone →map_err), differing only in the Wasm type and its name string. A generic helper parameterized on the target type would eliminate this ~40-line duplication. Note thatfee_strategy_from_js_optionsalready uses thetry_from_options_optional_with+try_to_arrayutils for similar work but with optional semantics.Not blocking — deferring is fine if the current clarity is preferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/src/platform_address/input_output.rs` around lines 186 - 265, Extract a generic helper (e.g., items_from_js_options<T>) that encapsulates the common scaffolding in inputs_from_js_options and outputs_from_js_options: read the property via js_sys::Reflect::get, check undefined/null, ensure it's an array, iterate/enumerate, call item.to_wasm::<T>(type_name) and clone the inner value, and map errors with the same "{}[{}] is not a <Type>" message; make the helper accept the JsValue options, field_name: &str, type_name: &str and a generic T used in to_wasm (with the same trait bounds required for cloning), then replace inputs_from_js_options and outputs_from_js_options to call this helper and return its result (keep the existing WasmDppError messages and signatures).packages/wasm-dpp2/tests/unit/AddressFundingFromAssetLockTransition.spec.ts (1)
164-172: Consider asserting the outputamountafter the setter.The
outputssetter test verifies the array length but not the updatedamountvalue (BigInt(50000)). A value assertion would strengthen this test.✏️ Suggested addition
transition.outputs = [newOutput]; const outputs = transition.outputs; expect(outputs).to.have.lengthOf(1); + expect(outputs[0].amount).to.equal(BigInt(50000));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/tests/unit/AddressFundingFromAssetLockTransition.spec.ts` around lines 164 - 172, The test currently only checks the outputs array length; after setting transition.outputs to [newOutput] also assert the amount on the stored output matches the intended BigInt(50000). Locate the test using createTransition(), new wasm.PlatformAddressOutput(newAddr, BigInt(50000)) and transition.outputs, then add an assertion that the retrieved outputs[0] (or outputs.at(0)) has an amount equal to BigInt(50000) to validate the setter preserved the value.packages/wasm-dpp2/tests/unit/IdentityCreateFromAddressesTransition.spec.ts (1)
81-100: MissingtoHex/fromHexround-trip coverage — same gap asIdentityTopUpFromAddressesTransition.spec.ts. Consider adding atoHex() / fromHex()describe block for consistency with the other transition suites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/tests/unit/IdentityCreateFromAddressesTransition.spec.ts` around lines 81 - 100, Add missing hex round-trip tests for IdentityCreateFromAddressesTransition: add a describe('toHex() / fromHex()') block mirroring the existing toBase64/toBytes tests, create a transition via createTransition(), call transition.toHex(), verify Buffer.from(hex, 'hex') equals Buffer.from(transition.toBytes()), then restore with wasm.IdentityCreateFromAddressesTransition.fromHex(hex) and assert Buffer.from(restored.toBytes()) equals the original bytes; reference the existing createTransition(), transition.toHex(), wasm.IdentityCreateFromAddressesTransition.fromHex(), transition.toBytes(), and restored.toBytes() methods when locating where to insert the new test.packages/wasm-dpp2/tests/unit/IdentityTopUpFromAddressesTransition.spec.ts (1)
76-95: MissingtoHex/fromHexround-trip coverage.The other transition spec files (
AddressFundsTransferTransition.spec.ts,IdentityCreditTransferToAddresses.spec.ts,AddressFundingFromAssetLockTransition.spec.ts) all include atoHex() / fromHex()suite. Consider adding one here for consistency:✏️ Suggested addition
+ describe('toHex() / fromHex()', () => { + it('should round-trip via hex', () => { + const transition = createTransition(); + const hex = transition.toHex(); + const bytes = transition.toBytes(); + + const restored = wasm.IdentityTopUpFromAddressesTransition.fromHex(hex); + expect(Buffer.from(restored.toBytes())).to.deep.equal(Buffer.from(bytes)); + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/tests/unit/IdentityTopUpFromAddressesTransition.spec.ts` around lines 76 - 95, Add a toHex()/fromHex() round-trip test for IdentityTopUpFromAddressesTransition similar to other transition specs: create a transition via createTransition(), call transition.toHex(), assert Buffer.from(hex, 'hex') equals Buffer.from(transition.toBytes()), then restore with wasm.IdentityTopUpFromAddressesTransition.fromHex(hex) and assert Buffer.from(restored.toBytes()) equals the original bytes; place this in a new describe('toHex() / fromHex()') / it('should round-trip via hex') block alongside the existing toBytes/toBase64 tests.packages/wasm-dpp2/tests/unit/AddressCreditWithdrawalTransition.spec.ts (1)
78-97: MissingtoHex/fromHexround-trip coverage — same gap as two other spec files in this PR. Consider adding atoHex() / fromHex()describe block for consistency across the transition suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/tests/unit/AddressCreditWithdrawalTransition.spec.ts` around lines 78 - 97, Add a new describe block testing toHex()/fromHex() round-trip for AddressCreditWithdrawalTransition: call createTransition(), get hex via transition.toHex(), assert Buffer.from(hex, 'hex') equals Buffer.from(transition.toBytes()), then restore via wasm.AddressCreditWithdrawalTransition.fromHex(hex) and assert Buffer.from(restored.toBytes()) equals the original bytes; place the test alongside the existing toBytes/toBase64 blocks and use the same createTransition helper and wasm.AddressCreditWithdrawalTransition methods.packages/wasm-dpp2/src/platform_address/transitions/identity_create_from_addresses_transition.rs (2)
163-179: Same unnecessary clone pattern inset_public_keys.
keysis a localVecthat's not reused —into_iter()avoids the per-element clone.♻️ Suggested optimization
IdentityCreateFromAddressesTransition::V0(v0) => { - v0.public_keys = keys - .iter() - .map(|k| IdentityPublicKeyInCreation::from(k.clone())) + v0.public_keys = keys + .into_iter() + .map(|k| IdentityPublicKeyInCreation::from(k)) .collect();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/src/platform_address/transitions/identity_create_from_addresses_transition.rs` around lines 163 - 179, The setter set_public_keys currently iterates over the local Vec keys with keys.iter() and calls k.clone() for each element; replace that clone pattern by consuming the vector with keys.into_iter() and mapping each IdentityPublicKeyInCreationWasm directly into IdentityPublicKeyInCreation (i.e., use IdentityPublicKeyInCreation::from(k) inside the map) when assigning to v0.public_keys in the IdentityCreateFromAddressesTransition::V0 arm to avoid unnecessary per-element cloning of IdentityPublicKeyInCreationWasm.
94-105: Unnecessary.clone()—public_keysis not used after this point.
public_keys.iter().map(|k| k.clone().into())borrows the vec and clones each element. Sincepublic_keysis a local that's not used afterwards,.into_iter()avoids the clone.♻️ Suggested optimization
IdentityCreateFromAddressesTransitionV0 { - public_keys: public_keys.iter().map(|k| k.clone().into()).collect(), + public_keys: public_keys.into_iter().map(|k| k.into()).collect(), inputs: inputs_map,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/src/platform_address/transitions/identity_create_from_addresses_transition.rs` around lines 94 - 105, The public_keys vector is cloned unnecessarily when constructing IdentityCreateFromAddressesTransitionV0; update the construction in IdentityCreateFromAddressesTransitionWasm / IdentityCreateFromAddressesTransition::V0 / IdentityCreateFromAddressesTransitionV0 to consume public_keys instead of iterating and cloning it by replacing public_keys.iter().map(|k| k.clone().into()) with an into-iterator that converts each element (e.g., public_keys.into_iter().map(Into::into)), since public_keys is not used after this point.packages/wasm-dpp2/src/platform_address/transitions/address_funding_from_asset_lock_transition.rs (1)
153-204: Minor inconsistency:inputsuses manual match whileasset_lock_proofandoutputsuse accessor trait methods.The
inputsgetter/setter (lines 163–184) manually matches onV0, whileasset_lock_proofandoutputsuse theAddressFundingFromAssetLockTransitionAccessorsV0trait methods. This works fine, but if an accessor forinputsbecomes available on the trait in the future, consider aligning for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wasm-dpp2/src/platform_address/transitions/address_funding_from_asset_lock_transition.rs` around lines 153 - 204, The inputs getter/setter manually pattern-match on AddressFundingFromAssetLockTransition::V0 while asset_lock_proof and outputs use the AddressFundingFromAssetLockTransitionAccessorsV0 trait; change inputs() and set_inputs() to call the same accessor methods on self.0 (e.g., use AddressFundingFromAssetLockTransitionAccessorsV0::inputs and ::set_inputs or the impl-provided inputs()/set_inputs() methods) and convert between PlatformAddressInputWasm and the inner types as before so the code consistently uses the transition accessor API (update the inputs() getter to iterate over self.0.inputs() and the set_inputs() to call self.0.set_inputs(inputs_map)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/wasm-dpp2/src/platform_address/transitions/address_credit_withdrawal_transition.rs`:
- Around line 179-206: The setter AddressCreditWithdrawalTransition::set_output
currently calls PlatformAddressOutputWasm::into_inner(), which can panic if the
PlatformAddressOutput has no amount; replace this with a fallible conversion
(e.g., add and call a try_into_inner() on PlatformAddressOutputWasm that returns
WasmDppResult<PlatformAddressOutput>) or explicitly validate the output_wasm has
an amount before converting and return a WasmDppError (invalid_argument) instead
of panicking. Update callers that use into_inner() (including
outputs_to_btree_map and implementations in AddressFundsTransferTransition and
other transition setters) to use the new try_into_inner() or the
validation+error pattern so all setters return graceful WasmDppResult errors
instead of causing a WASM panic.
In `@packages/wasm-dpp2/src/platform_address/transitions/mod.rs`:
- Line 10: The import line uses the misspelled module name
address_funds_transfer_transsition (extra "s"), causing an unresolved import for
AddressFundsTransferTransitionWasm; change the module path to the correct
address_funds_transfer_transition so the pub use
AddressFundsTransferTransitionWasm; resolves to the declared module name.
---
Duplicate comments:
In
`@packages/wasm-dpp2/src/platform_address/transitions/identity_credit_transfer_to_addresses_transition.rs`:
- Around line 159-175: The setter set_recipient_addresses calls
outputs_to_btree_map which currently uses into_inner().expect() and can panic if
a PlatformAddressOutput lacks an amount; change outputs_to_btree_map to return a
Result<BTreeMap<...>, JsValue> (or custom error) instead of unwrapping, update
set_recipient_addresses (and the constructor that also calls
outputs_to_btree_map) to handle the Result by propagating the error to JS (e.g.,
return Result<(), JsValue> or use wasm_bindgen::throw_str) and validate each
PlatformAddressOutputWasm for a present amount before converting so no .expect()
is used. Ensure references are updated for outputs_to_btree_map,
set_recipient_addresses, and the constructor that calls outputs_to_btree_map.
---
Nitpick comments:
In `@packages/wasm-dpp2/src/platform_address/input_output.rs`:
- Around line 186-265: Extract a generic helper (e.g., items_from_js_options<T>)
that encapsulates the common scaffolding in inputs_from_js_options and
outputs_from_js_options: read the property via js_sys::Reflect::get, check
undefined/null, ensure it's an array, iterate/enumerate, call
item.to_wasm::<T>(type_name) and clone the inner value, and map errors with the
same "{}[{}] is not a <Type>" message; make the helper accept the JsValue
options, field_name: &str, type_name: &str and a generic T used in to_wasm (with
the same trait bounds required for cloning), then replace inputs_from_js_options
and outputs_from_js_options to call this helper and return its result (keep the
existing WasmDppError messages and signatures).
In
`@packages/wasm-dpp2/src/platform_address/transitions/address_funding_from_asset_lock_transition.rs`:
- Around line 153-204: The inputs getter/setter manually pattern-match on
AddressFundingFromAssetLockTransition::V0 while asset_lock_proof and outputs use
the AddressFundingFromAssetLockTransitionAccessorsV0 trait; change inputs() and
set_inputs() to call the same accessor methods on self.0 (e.g., use
AddressFundingFromAssetLockTransitionAccessorsV0::inputs and ::set_inputs or the
impl-provided inputs()/set_inputs() methods) and convert between
PlatformAddressInputWasm and the inner types as before so the code consistently
uses the transition accessor API (update the inputs() getter to iterate over
self.0.inputs() and the set_inputs() to call self.0.set_inputs(inputs_map)).
In
`@packages/wasm-dpp2/src/platform_address/transitions/identity_create_from_addresses_transition.rs`:
- Around line 163-179: The setter set_public_keys currently iterates over the
local Vec keys with keys.iter() and calls k.clone() for each element; replace
that clone pattern by consuming the vector with keys.into_iter() and mapping
each IdentityPublicKeyInCreationWasm directly into IdentityPublicKeyInCreation
(i.e., use IdentityPublicKeyInCreation::from(k) inside the map) when assigning
to v0.public_keys in the IdentityCreateFromAddressesTransition::V0 arm to avoid
unnecessary per-element cloning of IdentityPublicKeyInCreationWasm.
- Around line 94-105: The public_keys vector is cloned unnecessarily when
constructing IdentityCreateFromAddressesTransitionV0; update the construction in
IdentityCreateFromAddressesTransitionWasm /
IdentityCreateFromAddressesTransition::V0 /
IdentityCreateFromAddressesTransitionV0 to consume public_keys instead of
iterating and cloning it by replacing public_keys.iter().map(|k|
k.clone().into()) with an into-iterator that converts each element (e.g.,
public_keys.into_iter().map(Into::into)), since public_keys is not used after
this point.
In `@packages/wasm-dpp2/tests/unit/AddressCreditWithdrawalTransition.spec.ts`:
- Around line 78-97: Add a new describe block testing toHex()/fromHex()
round-trip for AddressCreditWithdrawalTransition: call createTransition(), get
hex via transition.toHex(), assert Buffer.from(hex, 'hex') equals
Buffer.from(transition.toBytes()), then restore via
wasm.AddressCreditWithdrawalTransition.fromHex(hex) and assert
Buffer.from(restored.toBytes()) equals the original bytes; place the test
alongside the existing toBytes/toBase64 blocks and use the same createTransition
helper and wasm.AddressCreditWithdrawalTransition methods.
In `@packages/wasm-dpp2/tests/unit/AddressFundingFromAssetLockTransition.spec.ts`:
- Around line 164-172: The test currently only checks the outputs array length;
after setting transition.outputs to [newOutput] also assert the amount on the
stored output matches the intended BigInt(50000). Locate the test using
createTransition(), new wasm.PlatformAddressOutput(newAddr, BigInt(50000)) and
transition.outputs, then add an assertion that the retrieved outputs[0] (or
outputs.at(0)) has an amount equal to BigInt(50000) to validate the setter
preserved the value.
In `@packages/wasm-dpp2/tests/unit/AddressFundsTransfer.spec.ts`:
- Around line 42-45: The test filename uses PascalCase
(AddressFundsTransfer.spec.ts) but should follow the repo's kebab-case rule;
rename the file to address-funds-transfer.spec.ts and update any imports or test
references that point to AddressFundsTransfer.spec.ts (and do the same for other
new test files in this PR) so test runner and imports resolve correctly; verify
references in test suites or any index files that mention
AddressFundsTransferTransition or the skipped
describe('AddressFundsTransferTransition (.build API)') remain valid after the
rename.
In `@packages/wasm-dpp2/tests/unit/IdentityCreateFromAddressesTransition.spec.ts`:
- Around line 81-100: Add missing hex round-trip tests for
IdentityCreateFromAddressesTransition: add a describe('toHex() / fromHex()')
block mirroring the existing toBase64/toBytes tests, create a transition via
createTransition(), call transition.toHex(), verify Buffer.from(hex, 'hex')
equals Buffer.from(transition.toBytes()), then restore with
wasm.IdentityCreateFromAddressesTransition.fromHex(hex) and assert
Buffer.from(restored.toBytes()) equals the original bytes; reference the
existing createTransition(), transition.toHex(),
wasm.IdentityCreateFromAddressesTransition.fromHex(), transition.toBytes(), and
restored.toBytes() methods when locating where to insert the new test.
In `@packages/wasm-dpp2/tests/unit/IdentityTopUpFromAddressesTransition.spec.ts`:
- Around line 76-95: Add a toHex()/fromHex() round-trip test for
IdentityTopUpFromAddressesTransition similar to other transition specs: create a
transition via createTransition(), call transition.toHex(), assert
Buffer.from(hex, 'hex') equals Buffer.from(transition.toBytes()), then restore
with wasm.IdentityTopUpFromAddressesTransition.fromHex(hex) and assert
Buffer.from(restored.toBytes()) equals the original bytes; place this in a new
describe('toHex() / fromHex()') / it('should round-trip via hex') block
alongside the existing toBytes/toBase64 tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
packages/wasm-dpp2/src/lib.rspackages/wasm-dpp2/src/platform_address/address.rspackages/wasm-dpp2/src/platform_address/fee_strategy.rspackages/wasm-dpp2/src/platform_address/input_output.rspackages/wasm-dpp2/src/platform_address/mod.rspackages/wasm-dpp2/src/platform_address/transitions/address_credit_withdrawal_transition.rspackages/wasm-dpp2/src/platform_address/transitions/address_funding_from_asset_lock_transition.rspackages/wasm-dpp2/src/platform_address/transitions/address_funds_transfer_transition.rspackages/wasm-dpp2/src/platform_address/transitions/identity_create_from_addresses_transition.rspackages/wasm-dpp2/src/platform_address/transitions/identity_credit_transfer_to_addresses_transition.rspackages/wasm-dpp2/src/platform_address/transitions/identity_top_up_from_addresses_transition.rspackages/wasm-dpp2/src/platform_address/transitions/mod.rspackages/wasm-dpp2/tests/unit/AddressCreditWithdrawalTransition.spec.tspackages/wasm-dpp2/tests/unit/AddressFundingFromAssetLockTransition.spec.tspackages/wasm-dpp2/tests/unit/AddressFundsTransfer.spec.tspackages/wasm-dpp2/tests/unit/AddressFundsTransferTransition.spec.tspackages/wasm-dpp2/tests/unit/IdentityCreateFromAddressesTransition.spec.tspackages/wasm-dpp2/tests/unit/IdentityCreditTransferToAddresses.spec.tspackages/wasm-dpp2/tests/unit/IdentityTopUpFromAddressesTransition.spec.ts
Comment on lines
+179
to
+206
| #[wasm_bindgen(getter = "output")] | ||
| pub fn output(&self) -> Option<PlatformAddressOutputWasm> { | ||
| let output = match &self.0 { | ||
| AddressCreditWithdrawalTransition::V0(v0) => &v0.output, | ||
| }; | ||
| output.map(|(address, credits)| PlatformAddressOutputWasm::new(address, credits)) | ||
| } | ||
|
|
||
| #[wasm_bindgen(setter = "output")] | ||
| pub fn set_output(&mut self, output: JsValue) -> WasmDppResult<()> { | ||
| let new_output = if output.is_undefined() || output.is_null() { | ||
| None | ||
| } else { | ||
| let output_wasm = output | ||
| .to_wasm::<PlatformAddressOutputWasm>("PlatformAddressOutput") | ||
| .map(|r| (*r).clone()) | ||
| .map_err(|_| { | ||
| WasmDppError::invalid_argument("'output' is not a PlatformAddressOutput") | ||
| })?; | ||
| Some(output_wasm.into_inner()) | ||
| }; | ||
| match &mut self.0 { | ||
| AddressCreditWithdrawalTransition::V0(v0) => { | ||
| v0.output = new_output; | ||
| } | ||
| } | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
into_inner() will panic in WASM if the output has no amount.
Line 198 calls output_wasm.into_inner() which uses .expect(...) internally. If a user constructs a PlatformAddressOutput without an amount and passes it to the output setter, this will panic rather than returning a graceful error to JavaScript.
Consider using a fallible conversion (e.g., a try_into_inner that returns WasmDppResult) or validating the amount before calling into_inner.
This same risk applies to outputs_to_btree_map used in AddressFundsTransferTransition and other transitions.
Proposed fix in input_output.rs (root cause)
- pub fn into_inner(self) -> (PlatformAddress, Credits) {
- (
- self.address.into(),
- self.amount.expect("amount is required for this operation"),
- )
- }
+ pub fn into_inner(self) -> (PlatformAddress, Credits) {
+ (
+ self.address.into(),
+ self.amount.expect("amount is required for this operation"),
+ )
+ }
+
+ pub fn try_into_inner(self) -> WasmDppResult<(PlatformAddress, Credits)> {
+ let amount = self.amount.ok_or_else(|| {
+ WasmDppError::invalid_argument("amount is required for this output")
+ })?;
+ Ok((self.address.into(), amount))
+ }Then callers like outputs_to_btree_map and the output setter can use try_into_inner() to return errors instead of panicking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/wasm-dpp2/src/platform_address/transitions/address_credit_withdrawal_transition.rs`
around lines 179 - 206, The setter AddressCreditWithdrawalTransition::set_output
currently calls PlatformAddressOutputWasm::into_inner(), which can panic if the
PlatformAddressOutput has no amount; replace this with a fallible conversion
(e.g., add and call a try_into_inner() on PlatformAddressOutputWasm that returns
WasmDppResult<PlatformAddressOutput>) or explicitly validate the output_wasm has
an amount before converting and return a WasmDppError (invalid_argument) instead
of panicking. Update callers that use into_inner() (including
outputs_to_btree_map and implementations in AddressFundsTransferTransition and
other transition setters) to use the new try_into_inner() or the
validation+error pattern so all setters return graceful WasmDppResult errors
instead of causing a WASM panic.
|
|
||
| pub use address_credit_withdrawal_transition::AddressCreditWithdrawalTransitionWasm; | ||
| pub use address_funding_from_asset_lock_transition::AddressFundingFromAssetLockTransitionWasm; | ||
| pub use address_funds_transfer_transsition::AddressFundsTransferTransitionWasm; |
Contributor
There was a problem hiding this comment.
Typo in module path causes a compile error.
address_funds_transfer_transsition (double s) does not match the module declared on Line 3 (address_funds_transfer_transition). Rust will emit error[E0432]: unresolved import when building from source.
🐛 Proposed fix
-pub use address_funds_transfer_transsition::AddressFundsTransferTransitionWasm;
+pub use address_funds_transfer_transition::AddressFundsTransferTransitionWasm;📝 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
| pub use address_funds_transfer_transsition::AddressFundsTransferTransitionWasm; | |
| pub use address_funds_transfer_transition::AddressFundsTransferTransitionWasm; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/wasm-dpp2/src/platform_address/transitions/mod.rs` at line 10, The
import line uses the misspelled module name address_funds_transfer_transsition
(extra "s"), causing an unresolved import for
AddressFundsTransferTransitionWasm; change the module path to the correct
address_funds_transfer_transition so the pub use
AddressFundsTransferTransitionWasm; resolves to the declared module name.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
JavaScript/TypeScript applications need WASM bindings for the 6 platform address-based state transitions to create, serialize, and
interact with them from the browser and Node.js.
What was done?
New transition bindings (
platform_address/transitions/)Added WASM bindings for all 6 address-based state transitions:
AddressFundsTransferTransition— transfer credits between platform addressesAddressCreditWithdrawalTransition— withdraw credits to Core via output scriptAddressFundingFromAssetLockTransition— fund platform addresses via asset lock proofsIdentityCreateFromAddressesTransition— create identity from platform address inputsIdentityTopUpFromAddressesTransition— top up identity from platform addressesIdentityCreditTransferToAddressesTransition— transfer credits from identity to platform addressesEach binding exposes:
toBytes/fromBytes,toBase64/fromBase64,toHex/fromHextoObject/fromObject,toJSON/fromJSON(viaimpl_wasm_conversions!)toStateTransition/fromStateTransitionfor enum wrappingSupporting changes
input_output.rs— addedPlatformAddressOutput::new/new_optional, helper functionsinputs_from_js_options,optional_output_from_js_options,fee_strategy_from_js_options,outputs_from_js_optionsfee_strategy.rs— addedFeeStrategyStepWasmwithdeductFromInput/reduceOutputconstructors andto_wasmconversionaddress.rs— addedPlatformAddressWasm::newconstructor,impl_wasm_type_info!lib.rs— exported new transition types andFeeStrategyStepWasmHow Has This Been Tested?
yarn workspace @dashevo/wasm-dpp2 test— 838 passing, 0 failing.Added 7 test files:
AddressFundsTransferTransition.spec.ts— constructor, bytes/base64/hex round-trips, inputs/outputs getters/setters,userFeeIncrease, StateTransition conversion
AddressCreditWithdrawalTransition.spec.ts— constructor (with/without output),inputs/output/outputScript/pooling/coreFeePerByte getters/setters, StateTransition conversion
AddressFundingFromAssetLockTransition.spec.ts— constructor with asset lock proof, inputs/outputs/assetLockProofgetters/setters, StateTransition conversion
IdentityCreateFromAddressesTransition.spec.ts— constructor with public keys, inputs getter/setter, StateTransition conversionIdentityTopUpFromAddressesTransition.spec.ts— constructor with identityId (string and Identifier), inputs/output/identityIdgetters/setters, StateTransition conversion
IdentityCreditTransferToAddresses.spec.ts— constructor, recipientAddresses/senderId/nonce/signature/signaturePublicKeyIdgetters/setters, StateTransition conversion
AddressFundsTransfer.spec.ts—FeeStrategyStepWasmdeductFromInput/reduceOutput testsNote:
toObject/fromObject/toJSON/fromJSONconversion tests deferred to a separate PR to address a pre-existingis_human_readablemismatch in theimpl_wasm_conversions!macro.Breaking Changes
None
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
Release Notes