refactor(sdk)!: comprehensive Evo SDK refactoring #2999
Conversation
📝 WalkthroughWalkthroughThis pull request upgrades Node.js from version 20 to 24, migrates ESLint from v8 to v9 with new configuration modules, removes legacy ESLint configuration files across packages, and updates dependencies including tar. The changes involve consolidating ESLint config into new eslint.config.mjs files while removing old .eslintrc files, plus updating linting rule references from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
thephez
changed the title
shumkov
changed the title
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
packages/wasm-dpp2/src/state_transitions/batch/token_payment_info.rs (1)
153-156: Fix copy-paste bug: calling wrong setter method.The
set_minimum_token_costfunction incorrectly callsset_maximum_token_coston the inner type, which would cause the minimum cost value to be set as the maximum cost instead.🐛 Proposed fix
#[wasm_bindgen(setter = "minimumTokenCost")] pub fn set_minimum_token_cost(&mut self, minimum_cost: Option<TokenAmount>) { - self.0.set_maximum_token_cost(minimum_cost); + self.0.set_minimum_token_cost(minimum_cost); }packages/wasm-dpp2/src/enums/batch/gas_fees_paid_by.rs (1)
52-55: Incorrect error message references "batch type" instead of "gas fees".The error messages say
"unknown batch type value"but this enum isGasFeesPaidBy. This appears to be a copy-paste error.Fix error message
- _ => Err(WasmDppError::invalid_argument(format!( - "unknown batch type value: {}", - enum_val - ))), + _ => Err(WasmDppError::invalid_argument(format!( + "unknown gas fees paid by value: {}", + enum_val + ))),Apply the same fix at line 64-67.
Also applies to: 64-67
packages/wasm-dpp2/src/tokens/configuration/distribution_recipient.rs (1)
45-55: recipientType should stay a stable variant name.
Returning"Identity(<id>)"couples type and value, which breaks checks likerecipientType === "Identity". The identifier is already exposed viavalue, so this should return just"Identity".Proposed fix
- TokenDistributionRecipient::Identity(identity) => { - format!("Identity({})", IdentifierWasm::from(identity).to_base58()) - } + TokenDistributionRecipient::Identity(_) => String::from("Identity"),packages/wasm-dpp2/src/state_transitions/base/state_transition.rs (1)
101-112: Changekey_typeparameter to useOption<KeyTypeWasm>to properly reflect optional behavior.The method currently accepts
key_type: JsValueand checksis_undefined()for the default, but this pattern doesn't ensure the generated TypeScript signature treatskeyTypeas optional. UseOption<KeyTypeWasm>instead, matching the pattern already established inverify_public_key(line 131-132) for optional parameters, then convert using.map_or()or similar to preserve theECDSA_SECP256K1default.packages/wasm-dpp2/src/data_contract/transitions/update.rs (1)
166-190: Inconsistent parameter ordering withcreate.rs.The parameter order here is
(full_validation, platform_version), but increate.rs(line 182-183) it's(platform_version, full_validation). This inconsistency could confuse API consumers.Suggested fix to align parameter ordering
#[wasm_bindgen(js_name = "getDataContract")] pub fn get_data_contract( &self, - full_validation: Option<bool>, platform_version: PlatformVersionLikeJs, + full_validation: Option<bool>, ) -> WasmDppResult<DataContractWasm> {packages/wasm-dpp2/src/asset_lock_proof/outpoint.rs (1)
106-128: Guard fromBytes against oversized input.Line 111 slices with
lenfrom user input;len > 36will panic in WASM. Prefer explicit validation and return a structured error.🐛 Suggested fix (validate length and return error)
#[wasm_bindgen(js_name = "fromBytes")] -pub fn from_bytes(buffer: Vec<u8>) -> OutPointWasm { +pub fn from_bytes(buffer: Vec<u8>) -> WasmDppResult<OutPointWasm> { + if buffer.len() != 36 { + return Err(WasmDppError::invalid_argument(format!( + "OutPoint bytes must be 36 bytes, got {}", + buffer.len() + ))); + } let mut out_buffer = [0u8; 36]; - let bytes = buffer.as_slice(); - let len = bytes.len(); - out_buffer[..len].copy_from_slice(bytes); - - OutPointWasm(OutPoint::from(out_buffer)) + out_buffer.copy_from_slice(&buffer); + + Ok(OutPointWasm(OutPoint::from(out_buffer))) } @@ - Ok(OutPointWasm::from_bytes(bytes)) + OutPointWasm::from_bytes(bytes) @@ - Ok(OutPointWasm::from_bytes(bytes)) + OutPointWasm::from_bytes(bytes)packages/wasm-dpp2/src/identity/partial_identity.rs (1)
279-323: Accept numeric string keys inObject::keysiteration
Object::keysreturns strings regardless of key type, sokey.as_f64()will always fail. Parse string keys (or numeric values if somehow present) intou32.🐛 Suggested fix (accept numeric string keys)
- let key_val = key.as_f64().ok_or_else(|| { - WasmDppError::invalid_argument("Key identifier must be numeric") - })?; - - if key_val > u32::MAX as f64 { - return Err(WasmDppError::invalid_argument(format!( - "Key id '{:?}' exceeds the maximum limit for u32.", - key.as_string() - ))); - } - - let key_id = KeyID::from(key_val as u32); + let key_id_u32: u32 = if let Some(n) = key.as_f64() { + if n.fract() != 0.0 || n < 0.0 || n > u32::MAX as f64 { + return Err(WasmDppError::invalid_argument( + "Key identifier must be a valid u32", + )); + } + n as u32 + } else if let Some(s) = key.as_string() { + s.parse().map_err(|_| { + WasmDppError::invalid_argument("Key identifier must be numeric") + })? + } else { + return Err(WasmDppError::invalid_argument("Key identifier must be numeric")); + }; + + let key_id = KeyID::from(key_id_u32); @@ - key_val as u32, message + key_id_u32, message
🤖 Fix all issues with AI agents
In `@packages/wasm-dpp2/src/core/pro_tx_hash.rs`:
- Around line 34-67: The TryFrom implementation for ProTxHashLikeJs currently
only routes to ProTxHashWasm::try_from(JsValue) and therefore rejects an input
that is already a ProTxHash JS object; add a fast-path that detects a JS object
convertible via the IntoWasm trait (as done in IdentifierWasm) and convert it
directly to ProTxHashWasm before falling back to the hex/Uint8Array parsing;
update TryFrom<ProTxHashLikeJs> for ProTxHashWasm to call IntoWasm::into_wasm
when the JsValue is an object representing a ProTxHash, and mirror the same
pattern in TryFrom<ProTxHashLikeNullableJs> path so ProTxHash JS objects and
nullable/empty-string cases are handled correctly.
In `@packages/wasm-dpp2/src/data_contract/document/model.rs`:
- Around line 173-210: The constructor currently converts revision_js via
as_f64() and casts to u64 which can silently accept fractional/invalid values;
replace that logic to call crate::utils::try_to_u64(revision_js) and wrap the
result with Revision::from(...) (mirror the pattern used in
partial_identity.rs). Ensure you reference the local variable revision_js,
remove the direct as_f64() cast, and propagate any error from try_to_u64 so
invalid/NaN/negative/fractional values are rejected.
In `@packages/wasm-dpp2/src/enums/keys/purpose.rs`:
- Around line 6-11: The extern type PurposeLikeJs references a missing
TypeScript type alias "PurposeLike"; add a TS_TYPES block that defines
PurposeLike (e.g., a union of allowed values or string | number as used in other
key enum files) so the generated bindings include the TypeScript declaration;
update the same file's wasm-bindgen TS_TYPES section to declare export type
PurposeLike = /* match other enums' pattern, e.g. */ "AUTH" | "ENCRYPT" |
string; ensuring the identifier PurposeLike matches the typescript_type used on
the extern type PurposeLikeJs.
In `@packages/wasm-dpp2/src/enums/keys/security_level.rs`:
- Around line 6-11: The extern type SecurityLevelLikeJs declares typescript_type
= "SecurityLevelLike" but no TypeScript definition is emitted; add a matching
typescript_custom_section for "SecurityLevelLike" (similar to
gas_fees_paid_by.rs and key_type.rs) so consumers get the TypeScript
alias/interface. Locate the extern type SecurityLevelLikeJs and add a
#[wasm_bindgen(typescript_custom_section = r#"/* TS_TYPES */ ..."#)] attribute
that defines the SecurityLevelLike type (matching expected shape/union used
elsewhere) to ensure the generated d.ts includes the declaration.
In `@packages/wasm-dpp2/src/epoch/extended_epoch_info.rs`:
- Around line 90-94: The use of Reflect::get in the ExtendedEpochInfo
constructor (e.g., the options_obj -> Reflect::get(...)->as_f64() chain that
produces index as u16) is incorrect to treat Reflect::get errors as "missing
key" because Reflect::get returns Ok(JsValue::UNDEFINED) for absent properties;
update each property extraction (e.g., the index flow) to explicitly check for
JsValue::UNDEFINED or call .is_undefined() on the result and return
WasmDppError::invalid_argument with a clear "index is required" message when
undefined, then convert the defined value to number (using as_f64() and map to
invalid_argument "index must be a number" if None); apply the same pattern for
all other Reflect::get uses in the constructor and keep error creation via
WasmDppError::invalid_argument for consistent messages.
In `@packages/wasm-dpp2/src/state_transitions/batch/token_base_transition.rs`:
- Around line 61-66: The current extraction of tokenContractPosition casts a f64
to u16 with `as u16`, which can silently truncate; replicate the safe pattern
used for `identityContractNonce` by retrieving the value from `options_obj`, get
it as f64, convert it to an integer using the existing `try_to_u64` helper (or
the same conversion function used for `identityContractNonce`), validate range,
then explicitly cast to u16 only after successful conversion to prevent
truncation or out-of-range values when setting `token_contract_position`.
- Around line 76-87: The code currently uses
Reflect::get(...).unwrap_or(JsValue::UNDEFINED), which hides property access
errors; change it to propagate Reflect::get errors instead of swallowing them:
call Reflect::get(&options_obj, &"usingGroupInfo".into()) and propagate any Err
via the function's Result (using ? or mapping the JsValue error into the
function error), then use the returned JsValue (check is_undefined to
distinguish absence) before converting with to_wasm on
GroupStateTransitionInfoWasm and converting into GroupStateTransitionInfo;
ensure you reference the using_group_info_js variable, Reflect::get, to_wasm,
GroupStateTransitionInfoWasm, and GroupStateTransitionInfo when making the
change.
In `@packages/wasm-dpp2/src/version.rs`:
- Around line 16-41: The numeric path for PlatformVersionLike is accepting f64s
but PlatformVersionWasm::try_from currently casts f64 to u8 directly; update
PlatformVersionWasm::try_from to validate numeric inputs like NetworkWasm does:
if the incoming JsValue is a number, ensure it is an integer (no fractional
part) and within the valid u8 range (0..=255) before casting, and return a
WasmDppError for non-integer or out-of-range values; locate the conversion logic
in PlatformVersionWasm::try_from (and the TryFrom<PlatformVersionLikeJs> impl
that delegates to it) and mirror the integer validation pattern used by
NetworkWasm.
🧹 Nitpick comments (29)
packages/wasm-dpp2/JSVALUE_TYPE_AUDIT.md (1)
8-8: Clarify category numbering for readability.
Starting at “Category 2” without a preceding “Category 1” is a bit confusing in a standalone doc; consider renaming to a neutral header (e.g., “Return Types Using JsValue”) or adding Category 1 for completeness.packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_emergency_action.rs (2)
48-51: Unnecessary clone of entire struct.
self.clone()clones the whole wrapper unnecessarily. The method only needs the inner value'spublic_note_owned().♻️ Suggested fix
#[wasm_bindgen(getter = "publicNote")] pub fn get_public_note(&self) -> Option<String> { - self.clone().0.public_note_owned() + self.0.public_note().cloned() }If
public_note_owned()requires ownership, consider usingself.0.public_note().cloned()or checking if there's a borrowing accessor available.
53-56: Asymmetric getter/setter types foremergencyAction.The getter returns
Stringwhile the setter acceptsTokenEmergencyActionWasm. This creates an inconsistent API. Compare withbasewhich usesTokenBaseTransitionWasmfor both getter and setter.Consider returning
TokenEmergencyActionWasmfrom the getter for consistency:♻️ Suggested fix
#[wasm_bindgen(getter = "emergencyAction")] - pub fn get_emergency_action(&self) -> String { - TokenEmergencyActionWasm::from(self.0.emergency_action()).into() + pub fn get_emergency_action(&self) -> TokenEmergencyActionWasm { + self.0.emergency_action().into() }packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_claim.rs (1)
75-90: Consider extracting duplicated conversion logic.Lines 83-86 duplicate the same conversion pattern from the constructor (lines 36-39). If this pattern appears elsewhere in the codebase, a small helper function could reduce repetition:
fn parse_distribution_type(value: &JsValue) -> WasmDppResult<TokenDistributionTypeWasm> { match value.is_undefined() { true => Ok(TokenDistributionTypeWasm::default()), false => TokenDistributionTypeWasm::try_from(value.clone()), } }This is a minor nit given it's only two occurrences in this file.
packages/wasm-dpp2/src/state_transitions/batch/prefunded_voting_balance.rs (1)
29-35: Unconventional Rust function nameconstructor.While the
#[wasm_bindgen(constructor)]attribute correctly enables JSnew ClassName()syntax, the Rust function nameconstructoris non-idiomatic. The standard Rust convention is to name this functionnew, which would still work correctly with the wasm_bindgen attribute.That said, if this is a deliberate PR-wide convention for consistency with the JS/TS API surface, it's acceptable.
packages/wasm-sdk/src/wallet/key_generation.rs (1)
118-133: Pre-allocate the vector to avoid small reallocations.
This is minor, but cheap and clean sincecountis bounded.♻️ Suggested tweak
- let mut pairs = Vec::new(); + let mut pairs = Vec::with_capacity(count as usize);packages/wasm-dpp2/src/tokens/configuration/pre_programmed_distribution.rs (1)
131-132: Consider validating that inner value is a Map.
Map::from(value)will attempt to create a Map from any JsValue. If the caller passes a malformed structure where the inner value is not a Map (e.g., an object or array), this won't fail immediately but will produce incorrect/empty results, making debugging difficult.♻️ Proposed validation
- let inner_map = Map::from(value); + let inner_map: Map = value.dyn_into().map_err(|_| { + WasmDppError::invalid_argument(format!( + "Expected Map for distribution amounts at timestamp '{}', got: {:?}", + timestamp_str, + value.js_typeof() + )) + })?;This requires adding
dyn_intofromJsCast(already imported).packages/wasm-dpp2/src/identity/transitions/masternode_vote_transition.rs (1)
116-132: Consider adding null checks for optional fields.The code checks for
undefinedbut notnullfor optional fields. While the TypeScript types don't allownull, JavaScript callers could still pass it:
- For
signaturePublicKeyId: passingnullwould trigger "must be a number" error (misleading but safe)- For
signature:Uint8Array::from(null)may produce unexpected resultsThis follows the existing codebase pattern, so it's a minor edge case, but adding explicit null checks would improve robustness.
♻️ Suggested improvement
- let signature_public_key_id: KeyID = if signature_public_key_js.is_undefined() { + let signature_public_key_id: KeyID = if signature_public_key_js.is_undefined() || signature_public_key_js.is_null() { 0 } else { ... }; - let signature: Vec<u8> = if signature_js.is_undefined() { + let signature: Vec<u8> = if signature_js.is_undefined() || signature_js.is_null() { Vec::new() } else { Uint8Array::from(signature_js).to_vec() };packages/wasm-dpp2/src/tokens/configuration/reward_distribution_type.rs (1)
60-82: Redundant clone onfunctionin each match arm.The
functionis already owned after destructuring fromself.0.clone(), so the additional.clone()on lines 66, 72, and 78 is unnecessary.♻️ Suggested refactor to remove redundant clones
#[wasm_bindgen(getter = "distribution")] pub fn distribution(&self) -> JsValue { match self.0.clone() { RewardDistributionType::BlockBasedDistribution { interval, function } => { JsValue::from(BlockBasedDistributionWasm { interval, - function: function.clone().into(), + function: function.into(), }) } RewardDistributionType::TimeBasedDistribution { interval, function } => { JsValue::from(TimeBasedDistributionWasm { interval, - function: function.clone().into(), + function: function.into(), }) } RewardDistributionType::EpochBasedDistribution { interval, function } => { JsValue::from(EpochBasedDistributionWasm { interval, - function: function.clone().into(), + function: function.into(), }) } } }packages/wasm-dpp2/src/epoch/finalized_epoch_info.rs (1)
182-243: Consider adding field context totry_to_u64error messages.When
try_to_u64fails (e.g., forfirstBlockTime), the error message fromtry_to_u64won't include the field name, making it harder for SDK users to identify which field caused the issue.Example improvement for one field:
let first_block_time = try_to_u64( Reflect::get(&options_obj, &"firstBlockTime".into()) .map_err(|_| WasmDppError::invalid_argument("firstBlockTime is required"))?, -)? ; +).map_err(|e| WasmDppError::invalid_argument(format!("firstBlockTime: {}", e)))?;packages/wasm-dpp2/src/state_transitions/batch/token_transition.rs (2)
148-185: Consider avoiding unnecessary clone in getter.The
transitiongetter callsself.clone().0which clones the entireTokenTransitionWasmstruct. If theFromimplementations for the specific transition Wasm types can work with references or if the innerTokenTransitioncan be cloned directly, this could be optimized.♻️ Potential optimization
#[wasm_bindgen(getter = "transition")] pub fn transition(&self) -> JsValue { - match self.clone().0 { + match &self.0 { TokenTransition::Burn(token_transition) => { - TokenBurnTransitionWasm::from(token_transition).into() + TokenBurnTransitionWasm::from(token_transition.clone()).into() } // ... similar for other variants } }This avoids cloning the wrapper struct and only clones the specific inner transition that's actually needed.
187-202: Consider deriving transition type numbers to prevent protocol contract misalignment.The numeric values 0-10 are hardcoded in the match statement without any compile-time enforcement that they stay synchronized with the
TokenTransitionenum variants. Since this is exposed as a WASM binding (JavaScript API), these numbers form part of the protocol contract. If enum variants are reordered or added, the numbers here won't automatically update, risking silent breakage.Consider using explicit enum discriminants (
#[repr(u8)]) or a constant mapping to ensure type numbers remain synchronized with enum changes.packages/wasm-dpp2/src/tokens/configuration/configuration_convention.rs (1)
82-121: Validatelocalizationsis a plain object before coercion.
Object::fromwill coerce primitives/arrays into objects, which can yield confusing errors. Consider rejecting non-object inputs up front to keep error messages clear and prevent accidental acceptance of invalid shapes.♻️ Suggested guard (plus import)
-use js_sys::{Object, Reflect}; +use js_sys::{Array, Object, Reflect};fn value_to_localizations( localizations_value: &JsValue, ) -> WasmDppResult<BTreeMap<String, TokenConfigurationLocalization>> { + if localizations_value.is_null() || localizations_value.is_undefined() { + return Err(WasmDppError::invalid_argument( + "localizations must be an object", + )); + } + if !localizations_value.is_object() || Array::is_array(localizations_value) { + return Err(WasmDppError::invalid_argument( + "localizations must be a Record<string, TokenConfigurationLocalization>", + )); + } let js_object = Object::from(localizations_value.clone()); let mut localizations = BTreeMap::new();packages/wasm-dpp2/src/state_transitions/batch/token_pricing_schedule.rs (1)
81-86: Consider using bigint for SetPrices values for consistency.
SinglePricereturns values asbigint(line 76), butSetPricesserializes values using.into()which produces a JavaScriptnumber. For large credit values that exceed JavaScript's safe integer range, this could cause precision loss.♻️ Suggested fix for consistency
for (key, value) in prices.iter() { Reflect::set( &price_object, &JsValue::from(key.to_string()), - &(*value).into(), + &JsValue::bigint_from_str(&value.to_string()), )packages/wasm-dpp2/src/epoch/extended_epoch_info.rs (1)
90-94: Silent truncation on numeric casts may lose precision.The
as u16,as u32casts will silently truncate values exceeding their ranges:
- Line 94:
indexcast tou16(max 65,535)- Line 111:
first_core_block_heightcast tou32(max ~4.2 billion)- Line 122:
protocol_versioncast tou32If a JS caller passes a value like
100000forindex, it will be silently truncated.Consider adding range validation, especially for
index:let index_f64 = index_js.as_f64() .ok_or_else(|| WasmDppError::invalid_argument("index must be a number"))?; if index_f64 < 0.0 || index_f64 > u16::MAX as f64 { return Err(WasmDppError::invalid_argument( format!("index must be between 0 and {}", u16::MAX) )); } let index = index_f64 as u16;Also applies to: 106-111, 118-122
packages/wasm-dpp2/src/state_transitions/batch/batched_transition.rs (1)
100-103: Consider simplifying the identifier conversion path.The current conversion goes through an intermediate
JsValue:let id_value: JsValue = contract_id.into(); let contract_id: Identifier = IdentifierWasm::try_from(&id_value)?.into();Based on the
TryFrom<IdentifierLikeJs> for Identifierimplementation inidentifier.rs(lines 46-51), this could be simplified to:let contract_id: Identifier = contract_id.try_into()?;♻️ Suggested simplification
#[wasm_bindgen(setter = "dataContractId")] pub fn set_data_contract_id(&mut self, contract_id: IdentifierLikeJs) -> WasmDppResult<()> { - let id_value: JsValue = contract_id.into(); - let contract_id: Identifier = IdentifierWasm::try_from(&id_value)?.into(); + let contract_id: Identifier = contract_id.try_into()?;packages/wasm-dpp2/src/voting/vote_poll.rs (1)
82-112: Consider validating that required fields are present, not just that Reflect::get succeeds.
Reflect::gettypically doesn't throw an error when a property doesn't exist—it returnsOk(JsValue::UNDEFINED). The current error messages suggest the fields are "missing" when Reflect fails, but Reflect failure usually indicates a different issue (e.g., the target is not an object).For robustness, consider explicitly checking if the retrieved values are undefined:
♻️ Suggested validation improvement
// Extract contractId (required) let js_contract_id = Reflect::get(&object, &JsValue::from_str("contractId")) .map_err(|e| WasmDppError::invalid_argument(format!("Missing contractId: {:?}", e)))?; + if js_contract_id.is_undefined() { + return Err(WasmDppError::invalid_argument("contractId is required")); + } let contract_id = IdentifierWasm::try_from(&js_contract_id)?.into(); // Extract indexValues (required) let js_index_values = Reflect::get(&object, &JsValue::from_str("indexValues")) .map_err(|e| WasmDppError::invalid_argument(format!("Missing indexValues: {:?}", e)))?; + if js_index_values.is_undefined() { + return Err(WasmDppError::invalid_argument("indexValues is required")); + }packages/wasm-dpp2/src/identifier.rs (1)
339-355: Consider simplifying the error mapping inidentifiers_from_js_array.The error is already a
WasmDppErrorfromIdentifierWasm::try_from, so wrapping it in anotherinvalid_argumentadds redundant context. The inner error already contains "Invalid identifier" messaging.♻️ Suggested simplification
pub fn identifiers_from_js_array(array: IdentifierLikeArrayJs) -> WasmDppResult<Vec<Identifier>> { let js_value: JsValue = array.into(); let js_array = js_sys::Array::from(&js_value); js_array .iter() - .map(|v| { - IdentifierWasm::try_from(v) - .map(Identifier::from) - .map_err(|err| { - WasmDppError::invalid_argument(format!("Invalid identifier: {}", err)) - }) - }) + .map(|v| IdentifierWasm::try_from(v).map(Identifier::from)) .collect() }packages/wasm-dpp2/src/tokens/configuration_change_item/items/new_tokens_destination_identity.rs (1)
2-20: Align optional identity semantics with the signature.
The function acceptsIdentifierLikeJs(required) but still treatsundefinedasNone, whilenullwill error. If the value is intentionally optional, consider usingIdentifierLikeOrUndefinedJs(orOption<IdentifierLikeJs>) so the signature matches behavior andnull/undefinedare handled consistently.Proposed refactor
-use crate::identifier::{IdentifierLikeJs, IdentifierWasm}; +use crate::identifier::{IdentifierLikeOrUndefinedJs, IdentifierWasm}; @@ #[wasm_bindgen(js_name = "NewTokensDestinationIdentityItem")] pub fn new_tokens_destination_identity_item( - identity_id: IdentifierLikeJs, + identity_id: IdentifierLikeOrUndefinedJs, ) -> WasmDppResult<TokenConfigurationChangeItemWasm> { - let id_value: JsValue = identity_id.into(); - let identity_id: Option<Identifier> = match id_value.is_undefined() { - true => None, - false => Some(IdentifierWasm::try_from(&id_value)?.into()), - }; + let identity_id: Option<Identifier> = identity_id.try_into()?;packages/wasm-sdk/src/queries/group.rs (2)
798-835: Consider hoisting the inline import to the top-level imports.The function correctly uses
identifiers_from_js_arrayfor batch conversion. However, the inlineusestatement on line 803 could be moved to the top-level imports (line 20) for consistency with Rust conventions and to avoid redundant imports if this pattern is used elsewhere.♻️ Suggested refactor
Move the import to the top of the file:
-use wasm_dpp2::identifier::{IdentifierLikeArrayJs, IdentifierLikeJs, IdentifierWasm}; +use wasm_dpp2::identifier::{identifiers_from_js_array, IdentifierLikeArrayJs, IdentifierLikeJs, IdentifierWasm};Then remove the inline import on line 803.
1092-1139: Duplicate inline import - reinforces need to hoist.This function duplicates the inline
use wasm_dpp2::identifier::identifiers_from_js_arraystatement. Moving it to the top-level imports would eliminate this duplication.packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_transfer.rs (1)
178-216: Consider handlingnullin addition toundefinedfor note setters.The setters for
sharedEncryptedNoteandprivateEncryptedNoteonly checkis_undefined(). In JavaScript, callers might passnullto explicitly clear a value. Currently, passingnullwould attempt conversion and likely fail.♻️ Suggested fix
pub fn set_shared_encrypted_note( &mut self, shared_encrypted_note: &JsValue, ) -> WasmDppResult<()> { let shared_encrypted_note: Option<SharedEncryptedNote> = - match shared_encrypted_note.is_undefined() { + match shared_encrypted_note.is_undefined() || shared_encrypted_note.is_null() { true => None, false => Some( shared_encrypted_note .to_wasm::<SharedEncryptedNoteWasm>("SharedEncryptedNote")? .clone() .into(), ), };Apply the same pattern to
set_private_encrypted_note.packages/wasm-dpp2/src/identity/transitions/credit_withdrawal_transition.rs (1)
218-231: Consider handlingnullfor output script setter.Similar to the token transfer note setters,
set_output_scriptonly checksis_undefined(). JavaScript callers might passnullto clear the output script.♻️ Suggested fix
pub fn set_output_script( &mut self, #[wasm_bindgen(unchecked_param_type = "CoreScript | undefined")] script: &JsValue, ) -> WasmDppResult<()> { - if script.is_undefined() { + if script.is_undefined() || script.is_null() { self.0.set_output_script(None); } else {packages/wasm-dpp2/src/identity/transitions/update_transition.rs (1)
227-233: Consider simplifying identifier conversion.The setter converts
IdentifierLikeJs→JsValue→IdentifierWasm. Based on theTryFrom<IdentifierLikeJs>impl inidentifier.rs(lines 39-42), you can convert directly.♻️ Simplified conversion
pub fn set_identity_identifier(&mut self, identity_id: IdentifierLikeJs) -> WasmDppResult<()> { - let id_value: JsValue = identity_id.into(); - let identity_id = IdentifierWasm::try_from(&id_value)?.into(); + let identity_id: dpp::platform_value::Identifier = identity_id.try_into()?; self.0.set_identity_id(identity_id); Ok(()) }packages/wasm-dpp2/src/public_key.rs (1)
88-117: Consider documenting the auto-detection behavior inset_inner.The setter infers compression from byte length (33 = compressed, 65 = uncompressed), which differs from the constructor's explicit
compressedparameter. This is pragmatic but could surprise callers. Consider adding a JSDoc comment to clarify this behavior.packages/wasm-dpp2/src/group/action_event.rs (1)
67-72: Consider returningOption<TokenEventWasm>for future-proofing.Currently
token_event()unconditionally returnsTokenEventWasmsince there's only one variant. If additional variants are added toGroupActionEventin the future, this method would need updating. ReturningOption<TokenEventWasm>would be more defensive.Optional: Return Option for variant-specific accessor
#[wasm_bindgen(js_name = "tokenEvent")] - pub fn token_event(&self) -> TokenEventWasm { + pub fn token_event(&self) -> Option<TokenEventWasm> { match &self.0 { - GroupActionEvent::TokenEvent(event) => TokenEventWasm::from(event.clone()), + GroupActionEvent::TokenEvent(event) => Some(TokenEventWasm::from(event.clone())), } }packages/wasm-dpp2/src/identity/model.rs (1)
122-129: Consider keeping a backward-compatiblegetPublicKeysalias.If older JS consumers rely on
getPublicKeys, an alias eases migration; otherwise please document the break.♻️ Optional compatibility alias
#[wasm_bindgen(getter = "publicKeys")] pub fn public_keys(&self) -> Vec<IdentityPublicKeyWasm> { self.0 .public_keys() .values() .map(|key| IdentityPublicKeyWasm::from(key.clone())) .collect() } + +#[wasm_bindgen(js_name = "getPublicKeys")] +pub fn get_public_keys(&self) -> Vec<IdentityPublicKeyWasm> { + self.public_keys() +}packages/wasm-dpp2/src/identity/public_key.rs (2)
124-128: Missing validation for non-string input types could cause silent failures.When the input is not a string,
js_sys::Uint8Array::new(&js_value)is called without verifying the input is actually aUint8Array. This can produce unexpected results:
- A number
ncreates a zero-filled array of lengthn- An object or
nullmay create an empty arrayWhile callers like
get_identity_by_public_key_hashvalidate the 20-byte length afterward, this function could silently accept invalid inputs and return misleading data.♻️ Suggested validation
} else { - // Try to convert as Uint8Array - let array = js_sys::Uint8Array::new(&js_value); - Ok(array.to_vec()) + // Try to convert as Uint8Array + if js_value.is_instance_of::<js_sys::Uint8Array>() { + let array = js_sys::Uint8Array::unchecked_from_js(js_value); + Ok(array.to_vec()) + } else { + Err(WasmDppError::invalid_argument( + "Public key hash must be a hex string or Uint8Array", + )) + } }
154-169: Consider clarifying error handling for missing required fields.
Reflect::getreturnsOk(JsValue::UNDEFINED)for missing properties, notErr. The current error mapping on lines 156, 161-163, and 168 only catches cases where the target isn't an object. Ifpurpose,securityLevel, orkeyTypeare missing, the error will come from the subsequentTryFromconversion (e.g.,PurposeWasm::try_from(undefined)), not from these error handlers.This isn't a bug since invalid inputs will still be rejected, but the error messages may differ from expectations.
♻️ Optional: Explicit undefined check
let js_purpose = Reflect::get(&object, &JsValue::from_str("purpose")) - .map_err(|e| WasmDppError::invalid_argument(format!("Missing purpose: {:?}", e)))?; + .unwrap_or(JsValue::UNDEFINED); + if js_purpose.is_undefined() { + return Err(WasmDppError::invalid_argument("Missing required field: purpose")); + } let purpose = PurposeWasm::try_from(js_purpose)?;
packages/wasm-dpp2/src/state_transitions/batch/token_base_transition.rs
Outdated
Show resolved
Hide resolved
packages/wasm-dpp2/src/state_transitions/batch/token_base_transition.rs
Outdated
Show resolved
Hide resolved
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
packages/wasm-dpp2/src/enums/batch/gas_fees_paid_by.rs (1)
59-68: Validate numeric JsValue before casting to enum.
as_f64()followed byas u8will silently coerce values like1.7to1and-1to255, producing incorrect variants or mismapped errors. Please validate that the number is finite and an integer within range before casting.🛠️ Proposed fix
- if let Some(enum_val) = value.as_f64() { - return match enum_val as u8 { - 0 => Ok(GasFeesPaidByWasm::DocumentOwner), - 1 => Ok(GasFeesPaidByWasm::ContractOwner), - 2 => Ok(GasFeesPaidByWasm::PreferContractOwner), - _ => Err(WasmDppError::invalid_argument(format!( - "unknown batch type value: {}", - enum_val - ))), - }; - } + if let Some(enum_val) = value.as_f64() { + if !enum_val.is_finite() || enum_val.fract() != 0.0 { + return Err(WasmDppError::invalid_argument(format!( + "unknown batch type value: {}", + enum_val + ))); + } + return match enum_val as i64 { + 0 => Ok(GasFeesPaidByWasm::DocumentOwner), + 1 => Ok(GasFeesPaidByWasm::ContractOwner), + 2 => Ok(GasFeesPaidByWasm::PreferContractOwner), + _ => Err(WasmDppError::invalid_argument(format!( + "unknown batch type value: {}", + enum_val + ))), + }; + }packages/wasm-dpp2/src/state_transitions/batch/token_pricing_schedule.rs (1)
78-96: Inconsistent JS number representation forCreditsvalues may cause precision loss.In the
SinglePricebranch (line 76),Creditsis correctly serialized as a bigint usingbigint_from_str. However, in theSetPricesbranch (line 85),(*value).into()convertsCredits(u64) to a JavaScript number viaJsValue::from(u64), which uses f64 internally. This can lose precision for values exceedingNumber.MAX_SAFE_INTEGER(2^53 - 1).For consistency and correctness, both branches should serialize
Creditsas bigint.Proposed fix
for (key, value) in prices.iter() { Reflect::set( &price_object, &JsValue::from(key.to_string()), - &(*value).into(), + &JsValue::bigint_from_str(&value.to_string()), ) .map_err(|err| {packages/wasm-dpp2/src/state_transitions/batch/token_payment_info.rs (1)
153-156: Bug:set_minimum_token_costcallsset_maximum_token_costinstead.This is a copy-paste error. The setter for
minimumTokenCostincorrectly mutates the maximum cost field.🐛 Proposed fix
#[wasm_bindgen(setter = "minimumTokenCost")] pub fn set_minimum_token_cost(&mut self, minimum_cost: Option<TokenAmount>) { - self.0.set_maximum_token_cost(minimum_cost); + self.0.set_minimum_token_cost(minimum_cost); }packages/wasm-dpp2/src/core/core_script.rs (1)
36-60: Add input validation for hash lengths to match PlatformAddress pattern.Both
from_p2pkhandfrom_p2shsilently truncate inputs longer than 20 bytes via.min(). This is inconsistent with similar functions inpackages/wasm-dpp2/src/platform_address/address.rs(from_p2pkh_hashandfrom_p2sh_hash), which validate the input length and return aWasmDppErrorif the hash is not exactly 20 bytes.If a caller passes an incorrect length (e.g., 32-byte SHA256 instead of 20-byte HASH160), the current implementation silently truncates without error, potentially hiding bugs. Add explicit length validation and return an error for incorrect sizes:
pub fn from_p2pkh(key_hash: Vec<u8>) -> WasmDppResult<Self> { if key_hash.len() != 20 { return Err(WasmDppError::invalid_argument( "key_hash must be exactly 20 bytes".to_string() )); } // ... rest of implementation }packages/wasm-dpp2/src/core/private_key.rs (1)
101-104: Alignto_hex()output case with the parallel method inpublic_key.rs, which returns lowercase hex.The
to_hex()method inprivate_key.rsusesCase::Upper(uppercase), while the parallelto_hex()method inpublic_key.rs(line 355) useshex::encode(), which defaults to lowercase. This inconsistency within the same module violates the principle of least surprise. Additionally, the codebase predominantly usesCase::Lowerfor hex encoding (3 out of 3 other instances). Change toCase::Lowerfor consistency with both the parallel method and codebase convention.packages/wasm-dpp2/src/tokens/configuration/distribution_function.rs (1)
68-103: Validatesteps_with_amountis an object before iterating.With an unchecked param type, non-object inputs can silently yield an empty map and an unexpected distribution function. Consider an explicit guard.
🛠️ Suggested validation guard
pub fn stepwise( #[wasm_bindgen(unchecked_param_type = "Record<string, bigint | number>")] steps_with_amount: JsValue, ) -> WasmDppResult<DistributionFunctionWasm> { - let obj = Object::from(steps_with_amount); + if steps_with_amount.is_null() + || steps_with_amount.is_undefined() + || !steps_with_amount.is_object() + { + return Err(WasmDppError::invalid_argument( + "steps_with_amount must be an object mapping step to amount", + )); + } + let obj = Object::from(steps_with_amount);packages/wasm-dpp2/src/state_transitions/batch/document_base_transition.rs (1)
60-101: Explicitly guard missingdocumentId/dataContractId.
Reflect::getreturnsundefinedwithout error when a property is missing. The current flow then defers toIdentifierWasm::try_from, which yields a less clear error. Add a direct null/undefined check for clearer diagnostics.🛠️ Suggested guard
let js_document_id = Reflect::get(&object, &JsValue::from_str("documentId")) .map_err(|e| WasmDppError::invalid_argument(format!("Missing documentId: {:?}", e)))?; +if js_document_id.is_null() || js_document_id.is_undefined() { + return Err(WasmDppError::invalid_argument("documentId is required")); +} let document_id = IdentifierWasm::try_from(&js_document_id)?.into(); let js_data_contract_id = Reflect::get(&object, &JsValue::from_str("dataContractId")) .map_err(|e| { WasmDppError::invalid_argument(format!("Missing dataContractId: {:?}", e)) })?; +if js_data_contract_id.is_null() || js_data_contract_id.is_undefined() { + return Err(WasmDppError::invalid_argument("dataContractId is required")); +} let data_contract_id = IdentifierWasm::try_from(&js_data_contract_id)?.into();packages/wasm-dpp2/src/identity/partial_identity.rs (1)
279-354: Add validation to reject negative or fractional key IDs before casting tou32.The code casts floats to
u32without validation, allowing fractional values to truncate silently (e.g.,1.5→1) and negative values to wrap, potentially causing key ID collisions. This pattern is already properly validated elsewhere in the codebase (seepackages/wasm-dpp2/src/core/network.rs).Proposed fix
let key_val = key.as_f64().ok_or_else(|| { WasmDppError::invalid_argument("Key identifier must be numeric") })?; + if key_val.fract() != 0.0 || key_val < 0.0 { + return Err(WasmDppError::invalid_argument( + "Key identifier must be a non-negative integer", + )); + } if key_val > u32::MAX as f64 { return Err(WasmDppError::invalid_argument(format!( "Key id '{:?}' exceeds the maximum limit for u32.", key.as_string() ))); }let key_val = key.as_f64().ok_or_else(|| { WasmDppError::invalid_argument("Key id must be a numeric value") })?; + if key_val.fract() != 0.0 || key_val < 0.0 { + return Err(WasmDppError::invalid_argument( + "Key id must be a non-negative integer", + )); + } if key_val > u32::MAX as f64 { return Err(WasmDppError::invalid_argument(format!( "Key id '{:?}' exceeds the maximum limit for u32.", key.as_string() ))); }packages/wasm-dpp2/src/data_contract/document/model.rs (1)
500-542:fromObjectalways usesPlatformVersion::latest()while similar methods accept a version parameter.Unlike
fromBytes,fromHex, andfromBase64which accept aplatform_versionparameter,fromObject(line 534) andfromJSON(line 587) hardcodePlatformVersion::latest(). This inconsistency could cause issues when deserializing documents created with older platform versions.Consider adding an optional
platform_versionparameter tofromObjectandfromJSONfor consistency with the binary deserialization methods.
🤖 Fix all issues with AI agents
In `@packages/wasm-dpp2/src/asset_lock_proof/outpoint.rs`:
- Around line 106-114: The from_bytes implementation on OutPointWasm must
validate the input length instead of unconditionally slicing; change from_bytes
to return a Result (e.g., Result<OutPointWasm, JsValue>), check that
buffer.len() == 36 and return an Err with a clear message for invalid lengths,
otherwise construct the 36-byte array and create OutPoint::from; also update the
related from_hex and from_base64 helper functions to propagate this Result
instead of assuming success so callers receive proper errors.
In `@packages/wasm-dpp2/src/asset_lock_proof/proof.rs`:
- Around line 150-158: The getters instant_lock_proof and chain_lock_proof
currently call the panic-prone From<AssetLockProof> conversion (which panics for
the wrong variant); change each getter to perform a safe runtime check of the
inner AssetLockProof variant (use the same enum/method used by the From impl)
and return a non-panicking value: either return
Option<InstantAssetLockProofWasm>/Option<ChainAssetLockProofWasm> (annotated
with #[wasm_bindgen(js_name = ...)] so JS sees null when absent) or return
JsValue::NULL when the variant doesn't match; ensure you do not call the From
conversion unless the variant matches and reference the AssetLockProof
enum/variant matching logic used in the From<AssetLockProof> implementation.
In `@packages/wasm-dpp2/src/data_contract/document/model.rs`:
- Around line 177-196: The Reflect::get calls (producing properties_js,
document_type_name retrieval, data_contract_id_js, owner_id_js) can return
JsValue::UNDEFINED for missing properties so add explicit undefined checks after
each Reflect::get (e.g., if properties_js.is_undefined() { return
Err(WasmDppError::invalid_argument("Missing 'properties' field")) } and
similarly for documentTypeName, dataContractId, ownerId) before further
conversions; for documentTypeName check undefined first then call
.as_string().ok_or_else(...) as currently done, and for data_contract_id_js and
owner_id_js return the specific "Missing '...'" error if undefined before
calling IdentifierWasm::try_from.
In `@packages/wasm-dpp2/src/data_contract/model.rs`:
- Around line 180-188: Reflect::get returns Ok(JsValue::UNDEFINED) for missing
properties, so update the ownerId and schemas extraction to explicitly check the
returned JsValue for undefined (e.g., js_owner_id.is_undefined() /
js_schema.is_undefined()) and return a clear WasmDppError::invalid_argument when
missing; for ownerId keep using try_into() only after confirming js_owner_id is
not undefined, and for schemas call
serialization::platform_value_from_object(js_schema) only after confirming
js_schema is not undefined so you produce explicit "missing ownerId" / "missing
schemas" errors instead of obscure try_into/serialization errors.
In `@packages/wasm-dpp2/src/epoch/extended_epoch_info.rs`:
- Around line 106-122: The conversions for firstCoreBlockHeight and
protocolVersion currently cast f64 to u32 without bounds checks; add validation
like you did for index (or implement a reusable helper) to ensure the f64 is
finite, non-negative, an integer, and <= u32::MAX before casting. Update the
parsing of "firstCoreBlockHeight" and "protocolVersion" (and/or introduce a
helper used by both) to return a clear WasmDppError on invalid range or type
instead of silent truncation.
- Around line 90-94: The current extraction of index via
Reflect::get(...).as_f64().ok_or(...)? as u16 can silently truncate large or
non-integer JS numbers; modify the logic that reads `index` from `options_obj`
(the Reflect::get call and the `index` binding) to: convert to f64, verify it's
finite, an integer (e.g., floor equality), and within 0..=u16::MAX (65535); if
any check fails, return a WasmDppError::invalid_argument with a clear message,
otherwise cast safely to u16. This ensures `index` cannot overflow or be a
fractional/NaN/Infinity value when assigned to the `u16` variable.
In `@packages/wasm-dpp2/src/identity/public_key.rs`:
- Around line 115-129: The public_key_hash_from_js function accepts arbitrary
JsValue and can coerce non-Uint8Array values into empty arrays and doesn't
enforce HASH160's 20-byte length; update public_key_hash_from_js to first check
if js_value.as_string() yields a hex string and decode it, else verify
js_value.is_instance_of::<js_sys::Uint8Array>() before calling
js_sys::Uint8Array::new(&js_value), convert to Vec<u8>, and then validate the
resulting Vec length equals 20 (HASH160) returning a
WasmDppError::invalid_argument on failure; follow the same input-type check and
length validation pattern used in pro_tx_hash.rs to ensure callers no longer
need to duplicate this logic.
In `@packages/wasm-dpp2/src/identity/transitions/masternode_vote_transition.rs`:
- Around line 116-124: Replace the loose as_f64() parsing for
signaturePublicKeyId with the same robust validation used for nonce: call
try_to_u64 on signature_public_key_js (the variable currently named
signature_public_key_js) when it's not undefined, return a
WasmDppError::invalid_argument if conversion fails, then convert the resulting
u64 into the KeyID type (u32) with an explicit range check to ensure it fits;
update the variable signature_public_key_id to use this validated conversion so
fractional/negative/NaN/Infinity values are rejected consistently.
- Around line 99-115: After each Reflect::get call for pro_tx_hash_js,
voter_identity_id_js, vote_js and the nonce JsValue, add an explicit
undefined/null check (like the existing signaturePublicKeyId handling) and
return WasmDppError::invalid_argument("<field> is required") if the value is
undefined or null; then proceed to call IdentifierWasm::try_from for
pro_tx_hash_js and voter_identity_id_js, vote_js.to_wasm::<VoteWasm>("Vote") for
vote_js, and try_to_u64 for the nonce value so the conversions only run on
present values and produce your intended "X is required" errors.
In
`@packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_transfer.rs`:
- Around line 75-81: The code currently treats any non-string publicNote as
undefined and silently drops it; update the parsing logic around
Reflect::get(&options_obj, &"publicNote".into()) so that if
public_note_js.is_undefined() you return None, if public_note_js.is_string() you
set public_note = Some(public_note_js.as_string().unwrap()), and otherwise
return a descriptive type error (e.g., throw/return a Js TypeError) indicating
"publicNote must be a string" instead of converting to None; use the existing
variables public_note_js and public_note to locate and change the logic.
In `@packages/wasm-dpp2/src/tokens/configuration/distribution_rules.rs`:
- Around line 194-197: The getter name is inconsistent with the TypeScript
constructor option; change the wasm getter and Rust method to match the TS
property `mintingAllowChoosingDestination`: rename the #[wasm_bindgen(getter =
"...")] attribute and the Rust method `is_minting_allowing_choosing_destination`
(and any corresponding setter method/attribute) to use
`mintingAllowChoosingDestination`, and update any references to the old name so
the wasm interface and the TS constructor option use the same identifier.
- Around line 74-80: Reflect::get returns Ok(undefined) for missing properties,
so update the extraction for perpetual_distribution_rules (and the other
required fields with the same pattern) to explicitly check for undefined before
calling to_wasm: call Reflect::get(&options_obj,
&"perpetualDistributionRules".into())?, bind the JsValue, if
js_value.is_undefined() return
WasmDppError::invalid_argument("perpetualDistributionRules is required"),
otherwise call js_value.to_wasm::<ChangeControlRulesWasm>("ChangeControlRules")
and clone; apply the same pattern to the other fields referenced in this file
(the other Reflect::get usages around the noted line ranges) so missing
properties yield clear invalid_argument errors instead of relying on to_wasm.
In `@packages/wasm-dpp2/src/tokens/configuration/group.rs`:
- Around line 125-135: The get_members function currently returns a JS Map keyed
by Identifier JS objects which breaks value-based lookups; update the API so
keys are stable strings (e.g., base58 of the Identifier) or provide a helper
lookup method: modify get_members (and the GroupMembersMapJs return
type/contract) to insert keys as base58 strings via IdentifierWasm (use
IdentifierWasm -> base58 representation) instead of JS Identifier objects, or
alternatively add a new method like get_member_required_power(identifier:
&IdentifierWasm) / getMemberRequiredPower(identifier) that performs a
value-based lookup against the internal members map and returns the number, and
keep setMemberRequiredPower as-is to avoid reference-equality pitfalls.
In `@packages/wasm-dpp2/src/tokens/configuration/reward_distribution_type.rs`:
- Around line 144-148: The file contains duplicate impl blocks caused by the
macro invocations impl_wasm_type_info!(RewardDistributionTypeWasm,
RewardDistributionType); impl_wasm_type_info!(BlockBasedDistributionWasm,
BlockBasedDistribution); impl_wasm_type_info!(TimeBasedDistributionWasm,
TimeBasedDistribution); impl_wasm_type_info!(EpochBasedDistributionWasm,
EpochBasedDistribution); which duplicate the existing #[wasm_bindgen(js_class =
...)] impls for RewardDistributionTypeWasm, BlockBasedDistributionWasm,
TimeBasedDistributionWasm, and EpochBasedDistributionWasm; fix this by removing
the redundant macro invocations (or conversely remove the explicit
#[wasm_bindgen] impl blocks) so each type has exactly one impl block—preferably
delete the four impl_wasm_type_info!(...) lines to avoid conflicting
wasm_bindgen attributes.
In `@packages/wasm-dpp2/src/voting/vote_poll.rs`:
- Around line 87-98: Reflect::get does not error for missing properties, so
update the parsing of "contractId" and "indexValues" to explicitly check the
returned JsValue for undefined/null before attempting conversions: after calling
Reflect::get for "contractId" and "indexValues" validate with
JsValue::is_undefined() or is_null() and return a WasmDppError::invalid_argument
with a clear message if missing; only then call
IdentifierWasm::try_from(&js_contract_id) and
js_index_values.with_serde_to_platform_value()/into_array() to avoid downstream
panics and confusing errors.
In `@packages/wasm-sdk/src/queries/protocol.rs`:
- Around line 20-21: The field is_threshold_reached currently serializes as
"thresholdReached" (#[serde(rename = "thresholdReached")]) but the public getter
is named isThresholdReached, causing inconsistent external APIs between
toJSON()/serde output and the getter; pick one approach and make them
consistent: either rename the serde key to "isThresholdReached" or rename the
getter to thresholdReached (or add a documented alias) so both serde (the field
is_threshold_reached) and the public accessor (isThresholdReached or
thresholdReached) produce the same JSON/property name; apply the same fix for
the other occurrences referenced around lines 64–67 to keep naming consistent
across the struct.
🧹 Nitpick comments (25)
packages/wasm-dpp2/src/tokens/status.rs (1)
23-27: Potential JS API break:paused→isPausedrename.Line 23 renames the JS-visible getter. If backwards compatibility is required, consider keeping a deprecated alias (or ensure all downstream JS/TS usages and docs are updated in this PR).
♻️ Optional compatibility alias
impl TokenStatusWasm { #[wasm_bindgen(getter = "isPaused")] pub fn is_paused(&self) -> bool { match &self.0 { TokenStatus::V0(v0) => v0.paused(), } } + + // Deprecated alias for compatibility; remove in a future major release. + #[wasm_bindgen(getter = "paused")] + pub fn paused(&self) -> bool { + self.is_paused() + } }packages/wasm-dpp2/src/state_transitions/batch/token_transitions/set_price_for_direct_purchase.rs (1)
42-50: Consider handlingnullin addition toundefined.The constructor checks
price.is_undefined()to determine if a price was provided. In JavaScript, bothundefinedandnullare commonly used to indicate "no value". If a caller passesnull,is_undefined()returnsfalse, causing the code to attemptto_wasmconversion onnull, which will likely produce a confusing error.Proposed fix
- let price: Option<TokenPricingSchedule> = match price.is_undefined() { - true => None, - false => Some( + let price: Option<TokenPricingSchedule> = match price.is_undefined() || price.is_null() { + true => None, + false => Some( price .to_wasm::<TokenPricingScheduleWasm>("TokenPricingSchedule")? .clone() .into(), ), };packages/wasm-sdk/src/queries/mod.rs (1)
303-321: Consider using typed extern types for stricter TypeScript signatures.The change from
JsValuetojs_sys::Objectimproves type safety. However, the relevant code snippets inpackages/wasm-dpp2/src/asset_lock_proof/proof.rsshow usage of custom typed extern types (e.g.,AssetLockProofJSONJs,AssetLockProofObjectJs) which generate more descriptive TypeScript signatures.If full consistency with wasm-dpp2 patterns is desired, you could define typed extern types like:
#[wasm_bindgen] extern "C" { #[wasm_bindgen(typescript_type = "ProofMetadataResponseJSON")] pub type ProofMetadataResponseJSONJs; #[wasm_bindgen(typescript_type = "ProofMetadataResponseObject")] pub type ProofMetadataResponseObjectJs; }That said, using
js_sys::Objectis a valid intermediate step and the current implementation is functionally correct.packages/wasm-dpp2/src/tokens/configuration/configuration_convention.rs (1)
96-99: Consider adding input type validation.
Object::from(localizations_value.clone())will wrap anyJsValue(includingnull,undefined, or primitives) without error. If a non-object value is passed,Object::keys()will return an empty iterator, causing the function to silently return an emptyBTreeMapinstead of an error.While the
unchecked_param_typeattribute provides TypeScript compile-time checking, adding a runtime check would make debugging easier if invalid values slip through:🔧 Optional: Add type validation
fn value_to_localizations( localizations_value: &JsValue, ) -> WasmDppResult<BTreeMap<String, TokenConfigurationLocalization>> { + if !localizations_value.is_object() || localizations_value.is_null() { + return Err(WasmDppError::invalid_argument( + "Localizations must be an object", + )); + } let js_object = Object::from(localizations_value.clone()); let mut localizations = BTreeMap::new();packages/wasm-dpp2/src/state_transitions/base/state_transition.rs (1)
340-345: Consider accepting ownedJsValueto avoid clone.The parameter is
&JsValuebuttry_fromrequires an owned value, necessitating the clone. Acceptingpurpose: JsValuedirectly would be more efficient.♻️ Suggested change
#[wasm_bindgen(js_name = "getKeyLevelRequirement")] pub fn get_key_level_requirement( &self, - purpose: &JsValue, + purpose: JsValue, ) -> WasmDppResult<Option<Vec<String>>> { - let purpose = PurposeWasm::try_from(purpose.clone())?; + let purpose = PurposeWasm::try_from(purpose)?;packages/wasm-dpp2/src/epoch/finalized_epoch_info.rs (2)
236-238: Consider explicit Map type check for clearer error messages.
Map::from(block_proposers_js)wraps the JsValue without validating it's actually a Map. If a consumer bypasses TypeScript and passes a non-Map value, the error will occur during iteration with a less clear message.♻️ Suggested improvement
let block_proposers_js = Reflect::get(&options_obj, &"blockProposers".into()) .map_err(|_| WasmDppError::invalid_argument("blockProposers is required"))?; - let block_proposers = block_proposers_from_map(&Map::from(block_proposers_js))?; + let block_proposers_map = block_proposers_js + .dyn_into::<Map>() + .map_err(|_| WasmDppError::invalid_argument("blockProposers must be a Map"))?; + let block_proposers = block_proposers_from_map(&block_proposers_map)?;
382-391: Same Map validation suggestion applies here.For consistency and clearer error handling, consider using
dyn_into::<Map>()as suggested for the constructor.♻️ Suggested improvement
pub fn set_block_proposers( &mut self, block_proposers: BlockProposersMapJs, ) -> WasmDppResult<()> { - let block_proposers_map = - block_proposers_from_map(&Map::from(JsValue::from(block_proposers)))?; + let js_map = JsValue::from(block_proposers) + .dyn_into::<Map>() + .map_err(|_| WasmDppError::invalid_argument("blockProposers must be a Map"))?; + let block_proposers_map = block_proposers_from_map(&js_map)?; self.v0_mut().block_proposers = block_proposers_map; Ok(()) }packages/wasm-dpp2/src/tokens/configuration/keeps_history_rules.rs (1)
12-45: Consider rejecting unknown option fields to avoid silent typos.Right now, misspelled JS option keys are silently ignored and default to
false. If you want stricter input validation, consider denying unknown fields (confirm this matches desired JS leniency).♻️ Optional strict-validation tweak
-#[derive(Deserialize)] -#[serde(rename_all = "camelCase")] +#[derive(Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] struct TokenKeepsHistoryRulesOptions {packages/wasm-dpp2/src/voting/winner_info.rs (2)
7-33: Tighten TS typings foridentityIdbased onkind.
identityIdis optional for all kinds, so TS won’t enforce it forWonByIdentity. Consider a discriminated union to align with runtime validation.♻️ Proposed refactor
-export interface ContestedDocumentVotePollWinnerInfoObject { - kind: "NoWinner" | "WonByIdentity" | "Locked"; - identityId?: Uint8Array; -} +export type ContestedDocumentVotePollWinnerInfoObject = + | { kind: "NoWinner" | "Locked" } + | { kind: "WonByIdentity"; identityId: Uint8Array }; @@ -export interface ContestedDocumentVotePollWinnerInfoJSON { - kind: "NoWinner" | "WonByIdentity" | "Locked"; - identityId?: string; -} +export type ContestedDocumentVotePollWinnerInfoJSON = + | { kind: "NoWinner" | "Locked" } + | { kind: "WonByIdentity"; identityId: string };
53-78: RejectidentityIdfor non-identity kinds to avoid silent misuse.Right now,
identityIdis ignored forNoWinner/Locked, which can mask caller mistakes. Consider validating and erroring when it’s provided.♻️ Suggested validation
- match kind { - "NoWinner" | "noWinner" | "no_winner" | "none" | "NO_WINNER" => { + match kind { + "NoWinner" | "noWinner" | "no_winner" | "none" | "NO_WINNER" => { + if identity_id.is_some() { + return Err(JsValue::from_str( + "identityId must be omitted when kind is 'NoWinner'", + )); + } Ok(ContestedDocumentVotePollWinnerInfo::NoWinner.into()) } @@ - "Locked" | "locked" | "LOCKED" => { + "Locked" | "locked" | "LOCKED" => { + if identity_id.is_some() { + return Err(JsValue::from_str( + "identityId must be omitted when kind is 'Locked'", + )); + } Ok(ContestedDocumentVotePollWinnerInfo::Locked.into()) }packages/wasm-dpp2/src/tokens/configuration/pre_programmed_distribution.rs (2)
93-138: Validate nested map values before conversion.If a distribution entry value is not a
Map,Map::from(value)can lead to confusing runtime errors. A small guard improves error clarity.🔧 Proposed guard for nested map values
- let inner_map = Map::from(value); + if !value.is_instance_of::<Map>() { + return Err(WasmDppError::invalid_argument( + "Distribution amounts must be a Map", + )); + } + let inner_map = Map::from(value);
168-196: Guard against non-Map inputs in constructor/setter.
Map::fromuses unchecked casts; adding a fast validation improves error messages and avoids runtime TypeError on invalid inputs.🔧 Proposed guard for top-level map inputs
- let distributions_map = distributions_from_map(&Map::from(JsValue::from(distributions)))?; + let distributions_value = JsValue::from(distributions); + if distributions_value.is_null() + || distributions_value.is_undefined() + || !distributions_value.is_instance_of::<Map>() + { + return Err(WasmDppError::invalid_argument( + "distributions must be a Map", + )); + } + let distributions_map = distributions_from_map(&Map::from(distributions_value))?;- let distributions_map = distributions_from_map(&Map::from(JsValue::from(distributions)))?; + let distributions_value = JsValue::from(distributions); + if distributions_value.is_null() + || distributions_value.is_undefined() + || !distributions_value.is_instance_of::<Map>() + { + return Err(WasmDppError::invalid_argument( + "distributions must be a Map", + )); + } + let distributions_map = distributions_from_map(&Map::from(distributions_value))?;packages/wasm-dpp2/JSVALUE_TYPE_AUDIT.md (1)
3-41: Add concrete recommendations or tracking links to keep this audit actionable.Each item currently has an empty “Recommendation:” field. Consider adding a short recommendation, owner, or issue link (or mark TODO) so the checklist can be executed.
packages/wasm-dpp2/src/group/token_event.rs (1)
6-32: Consider more specific TypeScript interface definitions for better type safety.The current interfaces use
[key: string]: unknownwhich is very permissive. Given thatTokenEventis a discriminated union with known variants (Mint, Burn, Freeze, etc.), consider defining variant-specific properties or a discriminated union type for improved TypeScript type checking.For example:
export type TokenEventObject = | { variant: TokenEventVariant.Mint; amount: bigint; recipient: Uint8Array; ... } | { variant: TokenEventVariant.Burn; amount: bigint; ... } // ... other variantsHowever, if the loose typing is intentional for flexibility or due to implementation constraints, this is acceptable.
packages/wasm-dpp2/src/state_transitions/batch/token_transitions/token_emergency_action.rs (1)
53-71: Getter/setter type asymmetry may cause confusion in TypeScript.The
emergencyActiongetter returnsString(line 54-55) while the setter acceptsTokenEmergencyActionWasm(line 69). This asymmetry could lead to confusing TypeScript types where reading and writing the same property requires different types.Consider either:
- Having the getter return
TokenEmergencyActionWasmfor consistency, or- Having the setter accept
String(if it can be parsed back to the enum)♻️ Suggested fix: Return enum type from getter
#[wasm_bindgen(getter = "emergencyAction")] - pub fn get_emergency_action(&self) -> String { - TokenEmergencyActionWasm::from(self.0.emergency_action()).into() + pub fn get_emergency_action(&self) -> TokenEmergencyActionWasm { + TokenEmergencyActionWasm::from(self.0.emergency_action()) }packages/wasm-sdk/src/wallet/key_generation.rs (1)
115-135: Consider pre-allocating the vector capacity.The loop creates key pairs one at a time. Since
countis known upfront, pre-allocating would avoid reallocations.♻️ Suggested improvement
let network_wasm: NetworkWasm = network.try_into()?; let net: Network = network_wasm.into(); - let mut pairs = Vec::new(); + let mut pairs = Vec::with_capacity(count as usize); for _ in 0..count { pairs.push(Self::generate_key_pair_internal(net, &network_wasm)?); }packages/wasm-dpp2/src/state_transitions/batch/token_transition.rs (2)
148-185: Consider avoiding unnecessary clone intransition()getter.The getter uses
self.clone().0which clones the entire wrapper. Since you're only reading and converting the inner value, you could match on&self.0and clone only the specific variant data being returned.♻️ Suggested optimization
#[wasm_bindgen(getter = "transition")] pub fn transition(&self) -> JsValue { - match self.clone().0 { + match &self.0 { TokenTransition::Burn(token_transition) => { - TokenBurnTransitionWasm::from(token_transition).into() + TokenBurnTransitionWasm::from(token_transition.clone()).into() } // ... similar for other variants } }
187-221: Same unnecessary clone pattern intransition_type_number()andtransition_type().These getters only read the discriminant and don't need the inner data, so
self.clone().0is wasteful. Matching on&self.0would avoid the allocation.♻️ Suggested optimization
#[wasm_bindgen(getter = "transitionTypeNumber")] pub fn transition_type_number(&self) -> u8 { - match self.clone().0 { + match &self.0 { TokenTransition::Burn(_) => 0, TokenTransition::Mint(_) => 1, // ... etc } } #[wasm_bindgen(getter = "transitionType")] pub fn transition_type(&self) -> String { - match self.clone().0 { + match &self.0 { TokenTransition::Burn(_) => "Burn".to_string(), // ... etc } }packages/wasm-dpp2/src/state_transitions/batch/token_base_transition.rs (1)
163-171: Consider using conventional boolean ordering for readability.The match arms have inverted ordering (
false => Some,true => None) compared to the typical pattern. While correct, consider reordering for consistency with the constructor pattern at lines 78-87.♻️ Suggested reorder for consistency
let group_info: Option<GroupStateTransitionInfo> = match using_group_info.is_undefined() { - false => Some( - using_group_info - .to_wasm::<GroupStateTransitionInfoWasm>("GroupStateTransitionInfo")? - .clone() - .into(), - ), true => None, + false => Some( + using_group_info + .to_wasm::<GroupStateTransitionInfoWasm>("GroupStateTransitionInfo")? + .clone() + .into(), + ), };packages/wasm-dpp2/src/tokens/configuration/change_control_rules.rs (1)
116-135: Minor: Unnecessary clone in getters.The getters clone
self.0to match on it, which is functional but slightly inefficient. Consider borrowing instead if the enum supports it.♻️ Suggested improvement
- pub fn is_changing_authorized_action_takers_to_no_one_allowed(&self) -> bool { - match self.0.clone() { - ChangeControlRules::V0(v0) => v0.changing_authorized_action_takers_to_no_one_allowed, - } - } + pub fn is_changing_authorized_action_takers_to_no_one_allowed(&self) -> bool { + match &self.0 { + ChangeControlRules::V0(v0) => v0.changing_authorized_action_takers_to_no_one_allowed, + } + }Apply the same pattern to the other two getters.
packages/wasm-dpp2/src/core/pro_tx_hash.rs (1)
70-86: Consider removing redundant error wrapping.The error from
ProTxHashWasm::try_from(v)is already aWasmDppError. Wrapping it again withWasmDppError::invalid_argumentloses the original error details and creates a nested message.♻️ Suggested simplification
pub fn pro_tx_hashes_from_js_array( array: ProTxHashLikeArrayJs, ) -> Result<Vec<ProTxHash>, WasmDppError> { let js_value: JsValue = array.into(); let js_array = js_sys::Array::from(&js_value); js_array .iter() - .map(|v| { - ProTxHashWasm::try_from(v) - .map(ProTxHash::from) - .map_err(|err| { - WasmDppError::invalid_argument(format!("Invalid ProTxHash: {}", err)) - }) - }) + .enumerate() + .map(|(i, v)| { + ProTxHashWasm::try_from(v) + .map(ProTxHash::from) + .map_err(|err| { + WasmDppError::invalid_argument(format!("Invalid ProTxHash at index {}: {}", i, err)) + }) + }) .collect() }This adds the array index to the error message for better debugging while still providing context.
packages/wasm-dpp2/src/identifier.rs (1)
339-355: Consider simplifying the error message inidentifiers_from_js_array.The error wrapping adds "Invalid identifier:" but the underlying
WasmDppErrorfromIdentifierWasm::try_fromlikely already contains descriptive context. This may result in redundant messaging.♻️ Suggested simplification
js_array .iter() .map(|v| { IdentifierWasm::try_from(v) .map(Identifier::from) - .map_err(|err| { - WasmDppError::invalid_argument(format!("Invalid identifier: {}", err)) - }) }) .collect()Alternatively, keep the wrapper but make it add index context:
.enumerate() .map(|(i, v)| { IdentifierWasm::try_from(v) .map(Identifier::from) .map_err(|err| WasmDppError::invalid_argument(format!("Invalid identifier at index {}: {}", i, err))) })packages/wasm-sdk/src/queries/group.rs (1)
798-835: Consider moving the local import to module level.The
use wasm_dpp2::identifier::identifiers_from_js_array;import is duplicated insideget_groups_data_contractsandget_groups_data_contracts_with_proof_info. Moving it to the top-level imports (line 20) would be cleaner and avoid duplication.♻️ Suggested refactor
-use wasm_dpp2::identifier::{IdentifierLikeArrayJs, IdentifierLikeJs, IdentifierWasm}; +use wasm_dpp2::identifier::{IdentifierLikeArrayJs, IdentifierLikeJs, IdentifierWasm, identifiers_from_js_array};Then remove the local
usestatements fromget_groups_data_contracts(line 803) andget_groups_data_contracts_with_proof_info(line 1097).packages/wasm-dpp2/src/data_contract/model.rs (1)
193-197: Use logical OR (||) instead of bitwise OR (|) for boolean conditions.The expression
js_definitions.is_undefined() | js_definitions.is_null()uses bitwise OR which doesn't short-circuit. While functionally equivalent here since both sides are cheap boolean checks, using||is idiomatic and clearer for boolean logic.Suggested fix
let definitions: Option<Value> = - match js_definitions.is_undefined() | js_definitions.is_null() { + match js_definitions.is_undefined() || js_definitions.is_null() { true => None, false => Some(serialization::platform_value_from_object(js_definitions)?), };Same applies at line 203 for
js_tokens.packages/wasm-dpp2/src/data_contract/document/model.rs (1)
545-566:toJSONdoesn't accept aplatform_versionparameter unliketoObject.
toJSONusesPlatformVersion::latest()at line 548, whiletoObjectaccepts aplatform_versionparameter. For API consistency, consider adding the platform version parameter totoJSONas well.
packages/wasm-dpp2/src/tokens/configuration/distribution_rules.rs
Outdated
Show resolved
Hide resolved
Co-Authored-By: Claude Opus 4.5 <[email protected]>
…t scripts Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Change parameter js_name from 'binaryData' to 'signatureBytes' to avoid confusion with set_data's parameter in TypeScript. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/wasm-dpp2/src/identity/transitions/public_key_in_creation.rs`:
- Around line 227-232: Update the TypeScript declaration for
IdentityPublicKeyInCreationOptions so its data field is a string (hex encoded)
instead of Uint8Array to match how IdentityPublicKeyWasm::constructor is called;
locate the interface/type named IdentityPublicKeyInCreationOptions and change
the type of data to string with a comment like "// hex encoded", mirroring the
existing IdentityPublicKeyOptions in public_key.rs, and ensure any related
usages or JSDoc reflect the string/hex format.
Use try_from_options directly with options.as_ref() instead of converting to Object first. Implement TryFrom<&JsValue> for ContractBoundsWasm to enable using try_from_options_optional for contractBounds extraction. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Accept bytes directly instead of hex-encoded string for the data field. serde_wasm_bindgen handles Uint8Array to Vec<u8> conversion automatically. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Upgrade from ESLint 8.57.0 to ESLint 9.x with flat config format - Replace eslint-config-airbnb-base with eslint-config-airbnb-extended v3.0.1 - Create shared configs in eslint/ directory (base, typescript, wasm, mocha-tests) - Migrate all 24 packages to eslint.config.mjs flat config format - Delete all legacy .eslintrc* and .eslintignore files - Update inline eslint-disable comments (import/ to import-x/) - Fix code issues: unnecessary escapes in wasm-sdk, hex conversion in wasm-dpp2 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Update all packages to use explicit version constraints: - ESLint: * → ^9.18.0 (21 packages) - TypeScript: ^5.3.3 → ^5.7.3 (5 packages) Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Disable @stylistic/no-mixed-operators rule in base config - Add TypeScript support to wasm-dpp eslint config - Ignore generated protobuf code in dapi-grpc (clients/, src/) Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/dapi-grpc/eslint.config.mjs`:
- Around line 8-9: Update the misleading comment above the ignores array:
instead of saying "Generated protobuf code - ignore clients and src generated
directories", change it to accurately reflect what is being ignored by the rule
(e.g., "Ignore build artifacts, generated protobuf code, client bundles and
hand-written Rust sources (src/)") so it references the actual ignores array
(ignores: ['dist/**', 'node_modules/**', 'clients/**', 'src/**']) and clarifies
that src/** contains handwritten Rust files rather than generated protobuf code.
🧹 Nitpick comments (1)
packages/dashmate/eslint.config.mjs (1)
14-16: Consider placing global ignores at the start of the config array.In ESLint 9 flat config, placing an
ignores-only object at the end of the array creates a configuration-specific ignore pattern rather than a global ignore. For truly global ignores that apply before any other processing, it's recommended to place them at the beginning of the array.💡 Suggested reordering
export default [ + { + ignores: ['dist/**', 'node_modules/**'], + }, ...baseConfig, { files: ['**/*.js'], rules: { // Require .js extension for imports (ESM) 'import-x/extensions': ['error', 'ignorePackages', { js: 'always' }], }, }, mochaTestConfig, - { - ignores: ['dist/**', 'node_modules/**'], - }, ];
Addresses moderate severity deprecation warning in tar 7.5.x Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/dashmate/package.json`:
- Line 96: The package.json entry for the tar dependency uses a non-existent
version specifier "^7.6.0"; open packages/dashmate/package.json, locate the
"tar" dependency line and change the version to a valid published version (e.g.,
"^7.5.7" or the previously used "^7.5.4") so the constraint is satisfiable and
installs correctly, then update lockfile by running your package manager
(yarn/npm) to regenerate yarn.lock or package-lock.json.
… data Update test fixture to pass Uint8Array instead of hex string for the data field, matching the updated IdentityPublicKey constructor API. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Pin tar to exact version 7.5.7 in dashmate - Add tar resolution in root package.json - Addresses moderate severity advisory for tar 7.5.6 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Ensure options is properly converted to JsValue before passing to try_from_options functions to avoid Reflect.get on non-object errors. Co-Authored-By: Claude Opus 4.5 <[email protected]>
The lint script was accidentally emptied during the ESLint 9 migration. This restores it and adds eslint as a devDependency. Also fixes lint errors: adds missing Identity/Identifier imports, removes unused eslint-disable directives, and fixes unused catch variable. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ions Refactored 4 transition constructors to use serde for deserializing primitive fields (u16, u32, u64, Vec<u8>) instead of manual extraction. Complex types like IdentifierLike are still handled manually. This follows the pattern: serde for 3+ primitives, manual for *LikeJs types. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…ault handling Move undefined/null check into TryFrom<&JsValue> implementation to return default value, eliminating verbose 5-line patterns at 17+ call sites. Co-Authored-By: Claude Opus 4.5 <[email protected]>
thephez
changed the title
…ions Move IdentifierWasm fields into serde input structs, leveraging its custom Deserialize implementation to simplify constructors. Files updated: - identity transitions: credit_transfer, credit_withdrawal, update, masternode_vote - data_contract/document/model.rs - epoch/extended_epoch_info.rs - state_transitions/batch/token_payment_info.rs Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace Object::from with try_to_object for validating user input to ensure proper error handling when non-object values are passed. Also update try_to_* utility functions to accept impl Into<JsValue> for more flexible API. Co-Authored-By: Claude Opus 4.5 <[email protected]>
….clone() Reorder constructor operations to extract complex types first (borrowing &options), then call serde_wasm_bindgen::from_value(options.into()) last (consuming options). This eliminates unnecessary .clone() calls on options objects since try_from_options borrows while serde consumes. Co-Authored-By: Claude Opus 4.5 <[email protected]>
…tead of serde Remove IdentifierWasm and other wasm_bindgen types from serde input structs since serde deserialization cannot properly handle already- constructed wasm_bindgen instances passed from JavaScript. Pattern applied: extract complex types first via try_from_options (borrows &options), then deserialize primitive fields last via serde (consumes options). Co-Authored-By: Claude Opus 4.5 <[email protected]>
…e conversions
Replace verbose `to_wasm::<TypeWasm>("TypeName")?.clone()` patterns with
cleaner `TypeWasm::try_from(&value)?` by adding `impl_try_from_js_value!`
macro to wasm types and simplifying constructors to use `try_from_options`
and `try_from_options_optional` instead of `_with` variants.
- Add impl_try_from_js_value! to 20+ wasm types
- Simplify constructors using direct try_from_options calls
- Remove unused IntoWasm imports throughout
- Net reduction of ~140 lines of code
Co-Authored-By: Claude Opus 4.5 <[email protected]>
…nput types Replace inline `Type | string | number` patterns with centralized *Like type aliases for consistent TypeScript interfaces: - Add TokenDistributionTypeLike and TokenDistributionTypeLikeJs - Add ActionGoalLike and ActionGoalLikeJs - Update DataContractOptions to use PlatformVersionLike - Update TokenPaymentInfoOptions to use GasFeesPaidByLike - Update CanChangeAdminActionTakersOptions to use ActionGoalLike - Update TokenClaimTransition to use TokenDistributionTypeLike - Update IdentityPublicKeyInCreationOptions to use PurposeLike, SecurityLevelLike, KeyTypeLike Co-Authored-By: Claude Opus 4.5 <[email protected]>
Replace manual TryFrom and try_from_options implementations with impl_try_from_js_value! and impl_try_from_options! macros: - IdentitySignerWasm: use macros, remove 24 lines of boilerplate - PlatformAddressSignerWasm: use macros, remove 24 lines of boilerplate - PlatformAddressInput/Output: use try_to_u64 instead of local helper - Update 29 call sites in wasm-sdk to use new method signature Co-Authored-By: Claude Opus 4.5 <[email protected]>
Issue being fixed or feature implemented
Standardizes the wasm-dpp2 API with proper TypeScript types, improves developer experience with better error messages
What was done?
API Standardization (wasm-dpp2)
IdentifierLike,IdentifierLikeOrUndefined,NetworkLike,PlatformVersionLike)JsValuewith specific typed parameters in method signaturesIdentityJson,DocumentJson)get_prefix from getterstoJSON(),toObject(),fromJSON(),fromObject()methods consistently across typesUtility Functions (wasm-dpp2/wasm-sdk)
try_from_options,try_from_options_optional,try_from_options_withfor safe property extractiontry_to_u64,try_to_u32,try_to_u16,try_to_u8with string parsing and NaN/Infinity validationtry_to_string,try_to_bytes,try_to_array,try_to_vec,try_to_map,try_to_objectimpl_try_from_js_value!andimpl_wasm_type_info!macrosJsMapExttrait for DRY Map building patternsBug Fixes (wasm-dpp2)
from_bytesfunctions panicking on invalid input lengthReflect::getreturning undefined instead of error for missing propertiesProTxHashLikerejecting ProTxHash objectsassetLock→assetLockProof,recipientId→recipientOwnerId, etc.)&mut selfinstead of&selfDependencies
How Has This Been Tested?
describeblocks by method/feature$format_versionfield namesBreaking Changes
Dash Platform Protocol entities interface was changed.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.