-
Notifications
You must be signed in to change notification settings - Fork 155
[NEP-591]: Global Contracts #591
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
Conversation
aa2970e
to
650b881
Compare
650b881
to
39b0c80
Compare
39b0c80
to
b4df91c
Compare
b4df91c
to
6abff6a
Compare
6abff6a
to
424159e
Compare
Hi @pugachAG – thank you for starting this proposal. As the moderator, I labeled this PR as "Needs author revision" because we assume you are still working on it since you submitted it in "Draft" mode. |
Global contract can be deployed in 2 ways: either by its hash or by owner account id. | ||
Contracts deployed by hash are effectively immutable and cannot be updated. | ||
When deployed by account id the owner can redeploy the contract updating it for all its users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we talk about when to use each way? For instance, when should a user use hash instead of account id, and vice versa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, added in bfad2e3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to jump in, but I’m curious, will the deployed-by account ID receive a hash?
I don’t see a reason to allow someone to change my code and potentially take everything from my contract. But if a globally deployed account also gets a hash that I can lock onto (and update later if needed), that would be a nice DevX improvement.
I do see a downside, though that every update essentially creates a new copy of the code instead of replacing it. So I’m curious, how is this going to be addressed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akorchyn Nope, in this case we store account id, so the owner can update contract for all current users. There are underlying use cases for that, for example application developers onboarding users on NEAR.
Also it is possible to lock such contract if owner deletes all keys from the account, effectively making it impossible to make any changes.
@near/wg-protocol Can you please comment in the thread if you are leaning towards approving or rejecting this NEP? Please include your rationale and/or any feedback that you have for the authors. |
Such receipt contains destination shard `target_shard` as well as list of already visited shard ids `already_delivered_shards`. | ||
Applying `GlobalContractDistribution` receipt updates the corresponding `TrieKey::GlobalContractCode` in the trie for the current shard. | ||
It also generates distribution receipt for the next shard in the current shard layout. This process continues until `already_delivered_shards` contains all shards. | ||
This way we ensure that at any point in time there is at most one `GlobalContractDistribution` receipt in flight for a given deployment and eventually it will reach all shards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This way we ensure that at any point in time there is at most one
GlobalContractDistribution
receipt in flight
What is the motivation for having at most one such receipt, instead of deploying it in parallel to all shards, or at least using some other strategy that allows faster deployment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having at most one receipt allows us to easily handle edge case when deployment happens at the resharding boundary and we need to make sure the contract code reaches shards introduced in the new layout.
When deploying to all shards at once (in parallel) resharding would require either special handling or major changes to how we route buffered receipts. Specifically when buffered deployment receipt is targeted to an old shard, we can only re-route it to only one of the child shards after resharding, hence special logic would be required when applying that receipt to further forward it to the other child shard.
To summarise: we considered multiple options and decided to proceed with the current approach as it was the easiest to implement and reason about
|
||
### Costs | ||
|
||
For global contracts deployment we burn tokens for storage instead of locking like what we do regular contracts today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we burn the tokens for storage on every deployment? even when we are using deploying by account id which overrides and removes the previous one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, in the initial implementation we decided to proceed with the simplest approach and always burn tokens regardless of previously deployed global contract under that account id. This also indirectly covers distribution costs that cannot be fully accounted in gas costs due to chunk gas limit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous one isn't really removed. It will still be there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of deployment by account_id the previously deployed code will be replaced by the new one
## Security Implications | ||
|
||
One potential issue is increasing infrastructure cost for global contracts with growing number of shards. | ||
A global contract is effectively replicated on every shard, so with increase in number of shards each global contract uses more storage. | ||
This can be potentially addressed in the future by making deployment costs parametrized with the number of shards in the current epoch, but it still wouldn't address the issue for the already deployed contracts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Discussion] Presumably this will be a very popular feature that will get (ab)used a lot. If this is the case, and it accounts for a large percent of the state size, we should focus on strategies to prune/forget pieces of the state that can be recovered later.
|
||
`GlobalContractDistribution` receipt is generated as a result of processing `DeployGlobalContractAction`. | ||
Such receipt contains destination shard `target_shard` as well as list of already visited shard ids `already_delivered_shards`. | ||
Applying `GlobalContractDistribution` receipt updates the corresponding `TrieKey::GlobalContractCode` in the trie for the current shard. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, this means that a contract becomes available in a shard after processing the DeployGlobalContractAction
action, so the contract can be used in some shards before other ones right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case this is correct, some "funny" side effect, is that two contracts living in different shards, pointing at the same global contract, could run an action on the same block, and be using two different contracts, since the update is still in flight.
By "funny" I mean that it is hard to foresee side effects of this behavior, or if it could be exploited.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this can result in such "funny" behaviours. Effectively during upgrade 2 versions of the contract will be active on different shards for some range of blocks. That is something contract developers should be aware of. Bowen also raised that point and we reached a conclusion that it should not be a big issue in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats on delivering this NEP.
As a working group member I lean toward approving this NEP, and personally I'm looking forward to being released.
I left a few comments and questions.
As a working group member, I approve this NEP |
@flmel I guess this can be merged now? |
NEP Status (Updated by NEP Moderators)
Status: Approved
SME reviews:
Protocol Work Group voting indications (❔ | 👍 | 👎 ):
#591 (review)