Wasm misc tests & refactoring #1820
Conversation
🦋 Changeset detectedLatest commit: fafe2af The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| pub fn get_asset_id_inner(input_id_bin: &[u8]) -> WasmResult<Vec<u8>> { | ||
| pub fn get_asset_id(input_id_bin: &[u8]) -> WasmResult<Vec<u8>> { | ||
| let input_id = Id::decode(input_id_bin)?; | ||
| Ok(input_id.to_bytes().to_vec()) |
There was a problem hiding this comment.
In general, we should be relying on the buf build types and not rely upon the inner implementation details of the type. The former function specifically gave the bytes for the inner field. The fix simply just encodes to bytes for the entirety of the type.
packages/wasm/crate/src/auction.rs
Outdated
There was a problem hiding this comment.
Relocated types to dedicated test file
| note_object_store.create_index_with_params( | ||
| "nullifier", | ||
| &IdbKeyPath::new(nullifier_key), | ||
| web_sys::IdbIndexParameters::new().unique(false), |
There was a problem hiding this comment.
Clippy flagged unique() being a deprecated API and that set_unique() should be used instead
packages/wasm/crate/src/error.rs
Outdated
There was a problem hiding this comment.
A bunch of unused error types slipped in
| #![allow(dead_code)] | ||
| // Requires nightly. | ||
| #![cfg_attr(docsrs, feature(doc_auto_cfg))] | ||
|
|
||
| extern crate core; |
There was a problem hiding this comment.
Found this prevented clippy from finding dead code in the whole crate
packages/wasm/crate/src/metadata.rs
Outdated
There was a problem hiding this comment.
Tests relocated to dedicated test file
| epoch_duration: u64, | ||
| fvk: FullViewingKey, | ||
| notes: BTreeMap<note::StateCommitment, SpendableNoteRecord>, | ||
| swaps: BTreeMap<tct::StateCommitment, SwapRecord>, | ||
| denoms: BTreeMap<Id, Metadata>, |
There was a problem hiding this comment.
denoms and epoch_duration are unusued. Not sure why it's there 🤷
There was a problem hiding this comment.
regarding the epoch_duration field, this discord comment seems relevant.
now, when inspecting the ViewService protobuf definitions, I noticed that the epoch_index in the TransactionPlannerRequest is now deprecated. It seems that at some point, it may have been necessary for transaction planning before being deprecated possibly?
I traced the earlier view server wasm implementation back to last year, but I didn't find any useful comments on why these fields were introduced.
| epoch_duration: u64, | ||
| fvk: FullViewingKey, | ||
| notes: BTreeMap<note::StateCommitment, SpendableNoteRecord>, | ||
| swaps: BTreeMap<tct::StateCommitment, SwapRecord>, | ||
| denoms: BTreeMap<Id, Metadata>, |
There was a problem hiding this comment.
regarding the epoch_duration field, this discord comment seems relevant.
now, when inspecting the ViewService protobuf definitions, I noticed that the epoch_index in the TransactionPlannerRequest is now deprecated. It seems that at some point, it may have been necessary for transaction planning before being deprecated possibly?
I traced the earlier view server wasm implementation back to last year, but I didn't find any useful comments on why these fields were introduced.
A prelude in refining tracking issue: #691
Sort of a smorgasbord of random things for the wasm crate:
Afterward will be documenting in more detail in that tracking issue where we are missing tests