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.

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

[smart-contracts] Dictionary #39

Closed

Conversation

mpapierski
Copy link
Contributor

@mpapierski mpapierski commented Apr 1, 2021

@goral09 goral09 self-requested a review April 2, 2021 08:15
Comment on lines 142 to 145
[unresolved-questions]: #unresolved-questions

- How do we name this feature to reflect better what it does?
- Does it make sense to make the provisioned URef a unit? Can we use it to store additional information related to the stored data under the local key?
Copy link
Contributor

Choose a reason for hiding this comment

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

How does the migration of local storage look like? How will the upgraded contract access the pre-upgraded local data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This data migration will be transparent to already deployed contracts. I see this as an internal improvement, but the API would be forward-compatible.

During migration, we will iterate over reachable URefs (i.e., those stored under account or contract's named keys) detect if there's extra payload under that Unit with all the key bytes, then reindex the keys from the old hashed address (hash(uref_addr || key_bytes)), into uref_addr || hash(key_bytes) and remove the extra payload.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, thanks. I just wanted to make sure this is not being overlooked and we already have a plan for how to solve it. We can't deliver local keys without also allowing contract owners to transfer them between contract upgrades.


[drawbacks]: #drawbacks

As explained above current limitation of key lengths inside a trie requires us to maintain a secondary index of keys under a URef to retain the ability to migrate into the unhashed, more extended keyspace for more efficient iteration.
Copy link
Contributor

Choose a reason for hiding this comment

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

This restriction could be lifted. @henrytill is it worth getting support for variable length keys so we can avoid maintaining this secondary index, or should that be done at a different time?

@EdHastingsCasperAssociation EdHastingsCasperAssociation changed the title EE-1212: Local keys CEP-39: Local keys Jun 1, 2021
@EdHastingsCasperAssociation EdHastingsCasperAssociation changed the title CEP-39: Local keys Local keys Jun 1, 2021
@EdHastingsCasperAssociation EdHastingsCasperAssociation changed the title Local keys [smart-contracts] Dictionary Jun 10, 2021
casperlabs-bors-ng bot added a commit to casper-network/casper-node that referenced this pull request Jun 16, 2021
1467: EE-1242: Implement Dictionary r=dwerner a=mpapierski

This PR implements support for improved local keys, which intends to resolve past problems that lead to their removal. Contract developers will use this to store data in a separate keyspace prefixed by the URef address provisioned to a user by the system.

CEP for reference: casper-network/ceps#39

Closes #1420 

Co-authored-by: Michał Papierski <michal@casperlabs.io>
Co-authored-by: Henry Till <henrytill@gmail.com>
### dictionary_new

```rust
fn dictionary_new(name: &str) -> URef;
Copy link
Contributor

@goral09 goral09 Jun 23, 2021

Choose a reason for hiding this comment

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

In the implementation, it's new_dictionary.

fn dictionary_new(name: &str) -> URef;
```

Host provisions an URef of "unit" type under the hood used internally to secure access to the dictionary. Resulting URef will be automatically put into named keys, retaining his ability to add new data.
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if name is empty? Or if there already exists an entry with the same name in named keys collection of the account?

@RitaMAllenCA
Copy link

@darthsiroftardis is this still relevant or can it be merged? Please advise.....

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.

None yet

5 participants