Adds zcash_client_memory crate and minor changes to zcash_client_backend to support #1634
Conversation
This file contains 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
* add stubbed version of getting balances for wallet summary * adds helper methods at top level and partially implements filter for unspent notes * add map to store block end heights for tree shards * add orchard as well * accounts table * implement block_height_extrema * add impl of update_chain_tip * remove some unwraps from get_summary * add impls for get_max_checkpointed_height * add impl of get_target_and_anchor_heights * fix get_transaction * add impl for get_min_unspent_height * add get_memo impl * adds comment for truncate_to_height. Will implement this in future * tracing debug * yep * update last_Scanned_height outside tx loop * add updating orchard balance * clippy * add impls for inputSource trait * add sorting when selecting notes * Update Cargo.toml * serde block * allow constructing nullifiers * serde notes * serde transactions * top level in progress * scan queue * export error type * start account serde * serde account birthday * move serialization to separate module * clippy fix * skip * add unimplemented warnings for account import from seed phrase * remove unused imports * Remove Uivk * serde ufvk * Keep track of current diversifier idx * serde account * serde accounts * only serde left is on shard tree * almost done shardstore * use new trait * remote visit_seq from orchard note wrapper * remove unused array module * more nullifier to use trait * serialize shard store. need to deser now * serde memoryshardstore * serd shardtree * bring remaing code across for scanning * add orchard tree handling in put_blocks * only import if orchard enabled * Network Wrapper * done! cleanup time * clippy * break into modules * clippy fix * fmt * fmt * clean up frontierwrapper structs * add implementation of DataStoreFactor for memwallet * add back loading accounts from seeds only for testing * switch to use mutable ref in testing trait * partial implementation of scan_complete * fix bug from attepting to iterate over a reverse range in Rust * type safe tree and frontier serde * fmt * rename * more rename * fix issue with non-inclusive range * more cleanup * forgot to import * more clean * add proper recording of spend notes and memos * add partially implemented get_tx_history * ByteArray Trait * more refactor * clippy\ fix * Rename * cippy cix * proptest for shardtree serde * add fix for orchard support * more clean * donezo * clippy * add wasm-bindgen feature * move to upstream imt and shardtree patch * migrate 3 more tests across * adds new trait method to implement change_note_spends_succeed * Serde SendNoteTable * clippy fix * fmt * remove comment * Fix tree serialization when wallet has just been initialized * remove the raw field on get_tx_history as it broke tests * fix issue querying notes * add error handling * migrate send_multi_step_proposed_transfer * fix wallet deser * add data_api methods to allow fetching historical checkpoints * global fmt * fix clippy lints * fix test * add logic for detecting pending notes in wallet summary * fix serialization * fmt * change_note_spends_succeed passing for sapling! * add scan progress * add more comprehensive scan progress calculatioon * add unscanned ranges functionality * feature gate orchard import * add updating scan ranges on wallet import * fix bug in shard_end for updating ranges * passing birthday range scan check * cargo lock * screen out locked transactions when computing balance * fix missing nullifiers for orchard * retrievel all inputs in get_spendable_notes rather than filtering * rewrite note selection strategy to be split per pool * fix checkpoint getter for tests * fix clippy lints * switch from maybe_rayon to rayon for memwallet * remove proptest genereated file * move MemBlockCache to own module outside of testing * implement BlockCache and BlockSource * works! * change back batch size of batchrunners until further experimentation * TODO on for_each_checkpoint on sql store * switch from parking_lot to wasm_sync * add new records for transaction and add_partial method * mostly complete adding transparent output logic * remove debug * re add async trait * remove comment * test passing for shielding a transparent output * add start of implementation of data retrieval queue * add the extra features to wallet_read we need for tests * migrate tests to librustzcash and make generic over wallet impl * add test module for transparent tests * implement get_transparent_balance and get first transparent test passing * fix sqllite test * add truncate_to_height and gets transparent tests passing * global fmt * remove all IntoIterator impls * handle unwrap in get_spendable_transparent_outputs * serde on transparent related tables * fmt sqlite changes * remove incorrect todo * proto * make all methods in sync public * add from impl * adds serialization attributes to librustzcash types * format * add serialization to enable proposals to be serialized * put client memory back * add derives for noteId * add derives for FeeRule * add NoteSummary type to serialize notes * proto * fix transparent_balance_across_shielding * fix account delta * remove prints * handle ephemeral addrs * Add todo * default legacy transparent * filling in store_decrypted_tx * funding_accounts * multi_step passes * fmt * remove some prints * fmt * fmt * fmt * fix suggestions * ensure tests build with all feature flag combinations * removes serde skip from addresses field for an account and adds intermediate types to enable serialization * merge upstream and fix missing implementations. tests not passing * return a zero recovery ratio for now * fixing is_change error * fix nullifier spend detection logic * clippy lint * add tx account detection for utxo sending only. All tests passing * Willem's refactor * re-add .cargo * re-add editor config * keep willems branch with main * fix proto * remove whitespace --------- Co-authored-by: Willem Olding <[email protected]>
ec2
force-pushed
the
willem/zcash_client_memory
branch
from
8884660 to
c334f70
Compare
* Added account key sources and name with todos * implemented account_name into Account struct * implemented handling of key_source * fix test * lint
|
There's no CHANGELOG.md update neither one is created for the new crate |
pacu
reviewed
Dec 6, 2024
| @@ -2068,6 +2072,10 @@ impl AccountBirthday { | |||
| self.recover_until | |||
| } | |||
|
|
|||
| pub fn prior_chain_state(&self) -> &ChainState { | |||
There was a problem hiding this comment.
pub fn lacks documentation comment
| Default::default() | ||
| } | ||
|
|
||
| pub fn find_block(&self, block_height: BlockHeight) -> Option<CompactBlock> { |
There was a problem hiding this comment.
public method without documentation
| pub struct MemBlockCache(pub(crate) RwLock<BTreeMap<BlockHeight, CompactBlock>>); | ||
|
|
||
| impl MemBlockCache { | ||
| pub fn new() -> Self { |
There was a problem hiding this comment.
public method without documentation
| #[error("An error occurred while processing an account due to a failure in deriving the account's keys: {0}")] | ||
| BadAccountData(String), | ||
| #[error("Error converting byte vec to array: {0:?}")] | ||
| ByteVecToArrayConversion(Vec<u8>), |
There was a problem hiding this comment.
Would logging this error ever result in sensitive that being logged
| #[error("Balance error: {0}")] | ||
| Balance(#[from] zcash_protocol::value::BalanceError), | ||
| #[error("An error occurred while processing an account due to a failure in deriving the account's keys: {0}")] | ||
| BadAccountData(String), |
There was a problem hiding this comment.
Would logging this error ever result in sensitive that being logged
| address: &TransparentAddress, | ||
| tx_id: TxId, | ||
| ) -> Result<(), Error> { | ||
| // TODO: ephemeral_address_reuse_check |
There was a problem hiding this comment.
please create a github issue for this TODO and refer to it
| } | ||
|
|
||
| #[cfg(feature = "transparent-inputs")] | ||
| pub fn mark_ephemeral_address_as_used( |
There was a problem hiding this comment.
public function without documentation
| } | ||
|
|
||
| #[cfg(feature = "transparent-inputs")] | ||
| pub fn mark_ephemeral_address_as_seen( |
There was a problem hiding this comment.
public function without documentation
| #[cfg(feature = "transparent-inputs")] | ||
| pub fn mark_ephemeral_address_as_seen( | ||
| &mut self, | ||
| // txns: &TransactionTable, |
| ) -> Result<(), Error> { | ||
| for (idx, addr) in self.ephemeral_addresses.iter_mut() { | ||
| if addr.address == *address { | ||
| // TODO: this |
There was a problem hiding this comment.
Please create an issue for this TODO item if this is something out of the scope of this PR
pacu
reviewed
Dec 28, 2024
| protocol: zcash_protocol::ShieldedProtocol, | ||
| index: u32, | ||
| ) -> Result< | ||
| Option< |
There was a problem hiding this comment.
Why is this returning an option? Can't the None case be an error too?
| /// * the output is unspent as of the current chain tip. | ||
| /// | ||
| /// An output that is potentially spent by an unmined transaction in the mempool is excluded | ||
| /// iff the spending transaction will not be expired at `target_height`. |
| } | ||
|
|
||
| impl From<proto::Note> for Note { | ||
| fn from(note: proto::Note) -> Self { |
There was a problem hiding this comment.
This function apparently panics (probably for the right reasons). Shouldn't that be documented?
| use pretty_assertions::assert_eq; | ||
|
|
||
| #[test] | ||
| fn test_note_roundtrip() { |
There was a problem hiding this comment.
if panicking is a "feature" on this from function, shouldn't that be tested?
| /// A note that has been received by the wallet | ||
| /// TODO: Instead of Vec, perhaps we should identify by some unique ID | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| pub(crate) struct ReceivedNoteTable(pub(crate) Vec<ReceivedNote>); |
There was a problem hiding this comment.
[non-blocking]
@nuttycom what are the most common criteria to retrieve a note? I can think of a few
Global:
- by TxId
- By Account
By Account:
- Spend capability (Spent or Unspent)
- Anchor
A possible improvement would be holding an In-memory structure that can efficiently handle those
| /// A table of received notes. Corresponds to sapling_received_notes and orchard_received_notes tables. | ||
| #[derive(Clone, Debug, PartialEq)] | ||
| pub(crate) struct TransactionEntry { | ||
| // created: String, |
|
|
||
| // transparent_received_outputs | ||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
| pub struct ReceivedTransparentOutput { |
There was a problem hiding this comment.
public struct missing documentation
| } | ||
|
|
||
| impl ReceivedTransparentOutput { | ||
| pub fn new( |
| } | ||
| } | ||
|
|
||
| pub fn to_wallet_transparent_output( |
| pub struct TransparentSpendCache(pub(crate) BTreeSet<(TxId, OutPoint)>); | ||
|
|
||
| impl TransparentSpendCache { | ||
| pub fn new() -> Self { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
After a pretty big rewrite we have the memory wallet back-end in much better shape. In this large PR:
zcash_client_memorycratezcash_client_backendwasm-bindgenfeature flag which configures the time crate to support wasm buildsfinallymethod toWalletTestwhich allows wallet backend developers to add an extra hook that runs after each test to tidy up. We use it to test roundtrip serialization of the wallet after each testThis is a very large PR but it is almost entirely additions but expecting this to be a difficult one to review. Thanks in advance for anyone who takes it on!