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

Fix/Allow Native Contracts to be updated or receive new functions #2932

Closed
vncoelho opened this issue Oct 9, 2023 · 5 comments · Fixed by #2942
Closed

Fix/Allow Native Contracts to be updated or receive new functions #2932

vncoelho opened this issue Oct 9, 2023 · 5 comments · Fixed by #2942
Assignees

Comments

@vncoelho
Copy link
Member

vncoelho commented Oct 9, 2023

Currently, as described in #2910, updates on Native Contracts can be subjected to attacks if not precisely controlled with fork tags (maybe neo-vm modifications and updates may also led to such type of attacks).

For example:

  • Add new functions to CryptoLib or StdLib Native Contracts;

The attacker could use the future function and perform attacks such as turning a previous failed transaction into success (in case that node is re-synced), or even transforming success into failure.

A core problem of this issue are the re-syncs, which are currently part of a chosen design of Neo since Neo 2.

This first idea here is not to forbid re-syncs, but to create some HARD rules that should be respected in case they occurs, for example:

  • TX's can not change its state as well as the events it produces;

There are surely other rules that can be added.

In addition to that (or as an isolated option), there could be mechanisms for guiding Native Contracts to be upgraded with new features (as well as possible fixes), for example:

1- When new features are added to a Native Contracts, Consensus Nodes will need to agree and call an Upgrade Function that will add the new functions or update previous ones. The version of the Native Contract can be published along with the tx's in a compact manner. Native Contract versions would be kept and only called during resync.


There are possibilities for improving contracts but also to make resync to respect some key aspects of the Blockchain. In general, I am in favor that we never change State and do not even to check Witness again, what we need from the past is the result and its consequences. The latter should be respected.
But due to the design of NEO 2 and 3, maybe we can go in the direction of trying to handle that with more features.

@vncoelho
Copy link
Member Author

vncoelho commented Oct 9, 2023

@roman-khimov
Copy link
Contributor

We already have native contract versioning and we have hardforks, see #2915. In many ways on-chain trackable upgrades (via some committee-signed transaction) are better (hi, #1287), but they contradict hardfork logic we have and use currently, so I'd say it'd be easier for us to handle these problems with hardforks now (even though this means more and more clutter in the config).

That said, NeoFS sidechains are worrisome to me wrt new contracts, they do have an additional native contract and they have and use it since genesis. Which means that if we want C# node to get these contracts it'll have to enable them early as well. But if they're a part of some C-hardfork this would mean enabling C before B and A. Go figure.

@vncoelho
Copy link
Member Author

vncoelho commented Oct 9, 2023

Good, @roman-khimov, I had missed your recent issue. And regarding the @realloc discussion, I agree with all that points as well.

I think we should give priority to this and solve it for 3.7.0.
We can have some version in the meanwhile, like 3.6.2.
Or, we focus on a 3.7.0 just with the patches to handle this. Then, we add new features to Native Contracts, Neo-vm, etc...

@vncoelho
Copy link
Member Author

finally! Now we can easily update anything i guess

@vncoelho
Copy link
Member Author

vncoelho commented Feb 23, 2024

the vm jump table was also great

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 a pull request may close this issue.

5 participants