Skip to content

Added a limit value for scanning.#8

Merged
str4d merged 1 commit into
str4d:note-spending-v7from
gmale:feature/add-scan-limit
Mar 5, 2020
Merged

Added a limit value for scanning.#8
str4d merged 1 commit into
str4d:note-spending-v7from
gmale:feature/add-scan-limit

Conversation

@gmale
Copy link
Copy Markdown

@gmale gmale commented Feb 4, 2020

This provides a way to expose a more fine grained measure of scan progress. For example, by scanning in batches of 100 blocks, rather than everything that is pending.

Note: an alternative approach would be to not have 2 functions. I think I'm okay with the breaking changes that it would cause.

@gmale gmale force-pushed the feature/add-scan-limit branch 2 times, most recently from ec08f76 to 2eb9c52 Compare February 4, 2020 16:57
Copy link
Copy Markdown
Owner

@str4d str4d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squash the changes into the original commit.

Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this a u32 (LIMIT requires non-negative integers).

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could not get u32 to work with query_map I suspect this has to do with the underlying SQLite implementation only having signed integer types?

Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
Comment thread zcash_client_sqlite/src/scan.rs Outdated
@gmale gmale force-pushed the feature/add-scan-limit branch 2 times, most recently from 956feb9 to ab100d3 Compare February 5, 2020 21:42
This provides a way to expose a more fine grained measure of scan progress. For example, by scanning in batches of 100 blocks, rather than everything that is pending.
@gmale gmale force-pushed the feature/add-scan-limit branch from ab100d3 to a9364c5 Compare February 5, 2020 21:51

// Scan the cache again
scan_cached_blocks(db_cache, db_data).unwrap();
scan_cached_blocks(db_cache, db_data, None).unwrap();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be default Parameter so it's not a breaking change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the closest I could get to that. Rust doesn't do defaults in the same way swift or kotlin would. The alternative is a separate function but I'm leaning away from that since we're basically the only consumers of this. So it's safer to break things now than it will be next year :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and I think two functions just adds unnecessary complexity (one function for scanning is simpler)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, :( I totally agree on breaking things sooner than later

@gmale
Copy link
Copy Markdown
Author

gmale commented Feb 25, 2020

@str4d can this be merged? It seems like the errors are related to the param files (for macOS-latest) and json formatting (for Code coverage) and unrelated to the code changes in the PR.

I ask because I'm trying to set Linda up on the demo apps and she is getting build errors from not having this code.

@str4d str4d merged commit 902b9ff into str4d:note-spending-v7 Mar 5, 2020
str4d pushed a commit that referenced this pull request Dec 15, 2025
…14868de..23f0768ea

23f0768ea Release lightwallet-protocol v0.4.0
41156c767 Merge pull request #11 from zcash/feature/get_mempool_tx_pools
7c130e883 Add `lightwalletProtocolVersion` field to `LightdInfo` struct.
edbb726d7 Apply suggestion from code review
38fddd73b Apply suggestions from code review
0250f2720 Add pool type filtering to `GetMempoolTx` argument.
54ccaadd5 Change semantics of pool-based pruning of compact transactions from "may prune" to "must prune".
b0667ec99 Merge pull request #9 from zcash/2025-11-doc-TransparentAddressBlockFilter
f3fea7bd4 doc: TransparentAddressBlockFilter doesn't include mempool
a67dd323a Merge pull request #8 from zcash/2025-11-lightdinfo-upgrade-info
11da4b7e3 add next upgrade info to LightdInfo structure (GetLightdInfo)
42cd8f720 Transparent data docs update (#7)
c0cf957ac Merge pull request #5 from zcash/2025-11-comments
912fc3609 Minor clarification in GetBlockRange documentation.
6b03f2cce Documentation (comments) only
d978256a2 Merge pull request #1 from zcash/compact_tx_transparent
7eeb82e7c Merge pull request #4 from zcash/add_changelog
a95359dc9 Apply suggestions from code review
592b637a8 Add transparent data to the `CompactBlock` format.
9d1fb2c41 Add a CHANGELOG.md that documents the evolution of the light client protocol.
180717dfa Merge pull request #3 from zcash/merge_librustzcash_history
450bd4181 Merge the history of the .proto files from `librustzcash` for complete history preservation.
a4859d11d Move protobuf files into place for use in `zcash/lightwallet-protocol`
2e66cdd9e Update zcash_client_backend/proto/service.proto
eda012519 fix comment
f838d10ad Add gRPC LightdInfo Donation Address
db12c0415 Merge pull request zcash#1473 from nuttycom/wallet/enrichment_queue
698feba96 Apply suggestions from code review
20ce57ab3 zcash_client_backend: Add `block_height` argument to `decrypt_and_store_transaction`
a6dea1da8 Merge pull request zcash#1482 from zancas/doc_tweak
4d2d45fc9 fix incorrect doc-comment
e826f4740 update CompactBlock doc-comment, to cover non-Sapling shielded notes, and addresses
e9a6c00bf Various documentation improvements
988bc7214 Merge pull request zcash#872 from nuttycom/feature/pre_dag_sync-suggest_scan_ranges
58d07d469 Implement `suggest_scan_ranges` and `update_chain_tip`
a9222b338 Address comments from code review.
e20310857 Rename proto::compact::{BlockMetadata => ChainMetadata}
ac63418c5 Reorganize Sapling and Orchard note commitment tree sizes in CompactBlock.
0fdca14f1 zcash_client_backend: Add note commitment tree sizes to `CompactBlock` serialization.
2a0c2b8b7 zcash_client_backend: Add gRPC bindings behind feature flag
1342f0480 zcash_client_backend: Address compact_formats.proto comments
68aa4e01b zcash_client_backend: Bring in latest `compact_formats.proto`
e712eb1bc Add prevHash field to CompactBlock
440384c3e Build protobufs for compact formats

git-subtree-dir: zcash_client_backend/lightwallet-protocol
git-subtree-split: 23f0768ea4471b63285f3c0e9b6fbb361674aa2b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants