Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add lazy set collection #1196

Merged
merged 7 commits into from
Mar 13, 2023
Merged

add lazy set collection #1196

merged 7 commits into from
Mar 13, 2023

Conversation

tzemanovic
Copy link
Member

@tzemanovic tzemanovic commented Mar 7, 2023 •
edited
Loading

based on 0.14.1

related to #168 (we might also want to add key hashing variants of these later, if we have a use case for them)

@tzemanovic tzemanovic requested a review from brentstone March 7, 2023 10:12
@tzemanovic tzemanovic force-pushed the tomas/lazy-set branch 2 times, most recently from 79a05a2 to d6beb7b Compare March 7, 2023 10:20
@tzemanovic
Copy link
Member Author

pls update wasm

tzemanovic added a commit that referenced this pull request Mar 7, 2023
@tzemanovic
changelog: add #1196
fd287d1
@tzemanovic tzemanovic marked this pull request as ready for review March 7, 2023 11:25
@tzemanovic tzemanovic mentioned this pull request Mar 7, 2023
Merged
@brentstone
Copy link
Collaborator

I like this simple key method you have here that holds () values, but do you think it could still be useful to have another lazy set variant that particularly holds values / types that are Borsh-serializable and hashable? A la what is done in 7dd6508? In this commit, the lazy set is useful for holding types that are not going to implement the KeySeg trait, but they can be hashed, and so the set is effectively a map from a u64 hash value of a Borsh-serializable value to the value itself.

If this is desired, can call it LazyHashSet to distinguish with what is already in this PR. Then add to this PR.

@tzemanovic
Copy link
Member Author

I like this simple key method you have here that holds () values, but do you think it could still be useful to have another lazy set variant that particularly holds values / types that are Borsh-serializable and hashable? A la what is done in 7dd6508? In this commit, the lazy set is useful for holding types that are not going to implement the KeySeg trait, but they can be hashed, and so the set is effectively a map from a u64 hash value of a Borsh-serializable value to the value itself.

If this is desired, can call it LazyHashSet to distinguish with what is already in this PR. Then add to this PR.

Yeah, I've noted this in the issue #168 - I don't think we have a use case for it ourselves (unless it turns out to be more efficient than non-hashing version, which I don't foresee, but could be wrong). I think the use-case for these is like you say for types that cannot impl KeySeg and my feeling is that this will most likely come up with transactions/VPs programmability for 3rd party code with custom types

@tzemanovic
Copy link
Member Author

I'm gonna add try_insert here as suggested by @juped

@tzemanovic tzemanovic mentioned this pull request Mar 13, 2023
Merged
juped added a commit that referenced this pull request Mar 13, 2023
@juped
Sync with 0.14.2
22af8e1 
* tag 'v0.14.2':
  Namada 0.14.2
  changelog: add #1191
  test/pos/sm: fix init-validator and bond pre-conditions
  [ci] wasm checksums update
  ci: use nightly version for e2e test
  test/pos/sm: add the rest of the conditions
  pos: improve withdrawal logs
  test/pos/sm: add another bonds post-cond
  test/pos/sm: generate InitValidator transitions
  test/pos: fix the bonds test
  test/pos: reduce the bond token amounts to cover cases with same amounts
  bug fix: `update_validator_set` precisely checks if validator in consensus set
  pos: remove the `init` function to just use `set` instead
  make: add unstable-options to `check-abcipp` recipe
  pos: turn prints into tracing::debug, tidy up code
  pos/epoched: fix the update_data logic
  test/pos: add a state machine test
  test/core/address: fix address generator to be deterministic
  core/token: re-export `token::Change` type from storage_api mod
  make: use unstable-options to build unit tests
  changelog: add #1197
  [ci] wasm checksums update
  pos: ensure that validator consensus keys are unique
  core/storage: impl KeySeg for common::PublicKey
  test/lazy_set: add `try_insert` to state machine test
  core/lazy_set: add `try_insert` method
  small documentation edits
  changelog: add #1196
  [ci] wasm checksums update
  test: add a state machine test for lazy set collection
  core/storage_api: add LazySet
  changelog: #1182
  test/e2e: wait for a first block before client cmds
  wl_storage: remove commit_genesis method
  test/e2e: put ledger to bg to avoid it getting stuck
  gov/parameters: init via storage_api write log
  parameters: init chain parameters via storage_api write log
  init-chain: fix ibc to go via wl_storage
  [chore]:Added a doc warning
  [feat]: Dont' persist storage changes at genesis
  test/init_chain: ensure that init-chain doesn't commit to DB
  test/storage: reduce arb key length
  [ci] wasm checksums update
  changelog: add #1141
  bug fix: reliable deterministic ordering of keys in wl_storage PrefixIter that fixes apply_inflation bug
  test/core/wl_storage: add test for `prefix_iter_pre`/`prefix_iter_post`
@juped juped merged commit cf3b5e0 into main Mar 13, 2023
@juped juped deleted the tomas/lazy-set branch March 13, 2023 21:34
bengtlofgren pushed a commit that referenced this pull request May 11, 2023
@tzemanovic @bengtlofgren
changelog: add #1196
e92a884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brentstone brentstone Awaiting requested review from brentstone

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tzemanovic @brentstone @juped