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

Contract to manage contracts #2000

Closed
roman-khimov opened this issue Oct 12, 2020 · 7 comments · Fixed by #2119
Closed

Contract to manage contracts #2000

roman-khimov opened this issue Oct 12, 2020 · 7 comments · Fixed by #2119
Labels
discussion Initial issue state - proposed but not yet accepted
Milestone

Comments

@roman-khimov
Copy link
Contributor

Summary or problem description
We currently define state of the node (and network) as a set of contract storage key-value pairs for the purposes of #1383. Effectively, our state MPT includes all key-value pairs stored with the prefix of ST_Storage locally. This local DB stores a bit more data also, there are things like blocks/transactions/hash list and current block/header values, those are changed when persisting a block and directly reflect the contents of the chain. There are also two prefixes that stand out a little, ST_Contract and IX_ContractId, contract data and contract ID counter.

Values stored there are changed by executing VM code for transactions, so it's not the contents of the chain, it's something derived from it (like contract storage) and one can't just get it from the other node (like with blocks or transactions). Having or not having some contract in the storage obviously affects execution of subsequent transactions. If we're to imagine some state sharing mechanism (either MPT-based or one outlined in #1286), sharing contract storage DB only is not enough, deployed contracts should also be shared.

So technically because of these properties both contracts themselves and contract ID counter are a part of the network's state. And yet both are not a part of MPT for some reason, deploying a contract is not reflected there in any way.

Do you have any solution you want to propose?
This could be solved by adjusting MPT to include this data, but I think we can do a bit better than that.

I'm proposing moving contract management into another contract (that definitely should be the first (-1) native one). ID counter and contracts themselves would then just be a part of its state, stored with ST_Storage prefix locally and automatically accounted for in MPT. This would allow for better state tracking (in that deploying transactions would change MPT and state root hash), ability to prove correct deployment (via MPT proofs) and proper state exchange.

Current contract management syscalls could then become methods of this contract.

Neo Version

  • Neo 3

Where in the software does this update applies to?

  • Ledger
  • Native contracts
@roman-khimov roman-khimov added the discussion Initial issue state - proposed but not yet accepted label Oct 12, 2020
@roman-khimov
Copy link
Contributor Author

This also allows for proper notifications on contract creation/update. After #2044 contract updates might be more interesting to the outside world (seeing updates to contract you're using has some value) and thus it'd be nice for us to generate notifications in System.Contract.Create and System.Contract.Update. If we're to do this now these notifications would be emitted using deployment/update transaction script hash for ScriptHash property, making it hard to find them. Emitting notifications from contract-managing contract solves that, one could easily filter by its hash and receive appropriate events.

@erikzhang
Copy link
Member

If the deployment SYSCALL becomes the contract method, how to deploy itself by invoking its method before it is deployed?

@roman-khimov
Copy link
Contributor Author

Native contracts are deployed with Neo.Native.Deploy anyway, bypassing System.Contract.Create. And being native it can also do any sorts of magic in its Initialize during genesis block processing.

@erikzhang
Copy link
Member

Wait for #2056

@Tommo-L
Copy link
Contributor

Tommo-L commented Dec 2, 2020

Add one line? https://github.com/neo-project/neo-modules/pull/408/files#diff-c6a09595fd59bfc64002877307f9c029800bc17f8f6f1cf84e35db4ae953a14fR18

foreach (var item in snapshot.Contracts.GetChangeSet().Where(p => p.State != TrackState.None))

@Tommo-L
Copy link
Contributor

Tommo-L commented Dec 2, 2020

This also allows for proper notifications on contract creation/update.

Maybe we can create system notification for syscall.

@roman-khimov
Copy link
Contributor Author

Add one line?

  1. It technically can clash with keys used in Storages (keys there don't have prefixes yet)
  2. We really can do better than that

Maybe we can create system notification for syscall.

Well, we have a model where notifications are tied to some contracts, do we really have a reason to change that? I think we can keep it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Initial issue state - proposed but not yet accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants