Implement native Notary contract

[AnnaShaleva]

Description

Implement native Notary contract.

Close #2425. Depends on #3175 (will rebase this PR after #3175 merge). Tests are in progress, but the code is ready for review.

Type of change

• [X] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• [X] Native Gas contract unit tests;
• [X] Native Notary contract unit tests.

Checklist:

• [X] My code follows the style guidelines of this project
• [X] I have performed a self-review of my code
• [X] I have commented my code, particularly in hard-to-understand areas
• [X] I have made corresponding changes to the documentation
• [X] My changes generate no new warnings
• [X] I have added tests that prove my fix is effective or that my feature works
• [X] New and existing unit tests pass locally with my changes
• [ ] Any dependent changes have been merged and published in downstream modules:
  • [ ] neo-modules update
  • [ ] neo-devpack-dotnet-update

[vncoelho]

Is this PR to be merged now or after next release? I was planning to review current release and then merge these next feature.

[roman-khimov]

Is this PR to be merged now or after next release?

https://github.com/neo-project/neo/milestone/2

[dusmart]

This contract has a Verify method which is very dangerous. https://github.com/neo-project/neo/blob/77ffe094368d14b2f8c089e9b52479d6f1a8cc4b/src/Neo/SmartContract/Native/Notary.cs#L95

In this method, we verify that tx.Signers[1] has enough balance stored in the contract. Plus, we verify that a signature from any notary is provided.

Here is an attack vector. A malicious notary can sign as many transactions as it wants and then publish at most 500 txs on chain while tx.Signers[1] has a little balance only sufficient for one tx.

If it's not a malicious notary, someone could still get a chance to make the notary sign something multiple times and cache them then publish them at once.

By the way, do you know how many core modules will be affected by this notary feature? Is this the last PR?

[AnnaShaleva]

By the way, do you know how many core modules will be affected by this notary feature? Is this the last PR?

To be able to process existing NeoFS chains that use Notary - yes, it's the last PR. But ideally in future we'd like to implement P2PNotaryRequest payload with pool (ref. https://github.com/nspcc-dev/neo-go/issues/1546) and Notary service plugin (ref. https://github.com/nspcc-dev/neo-go/issues/1547). We'll add related issues to neo-project later.

"The contract itself don't send the transactions. It's a designated Notary node who is powered to send transactions on behalf of notary service users. " I want to check that because it should be, at least, similar to the oracles design.

It's a part of Notary service plugin. See the example implementation in https://github.com/nspcc-dev/neo-go/blob/master/pkg/services/notary/notary.go.

[roman-khimov]

verify method should be used with extreme caution

That's a valid concern, but there are multiple ways to handle it, we'll get to it after #3175 merge.

[AnnaShaleva]

and is there a fork introduced?

This PR changes the states, because we need Notary contract to be presented in the genesis block. And it's intentionally that we don't use Cockatrice to activate Notary contract.

[shargon]

because we need Notary contract to be presented in the genesis block

That is something that I still don't know why? why we need the notary from genesis?

[roman-khimov]

why we need the notary from genesis?

To be able to work with these networks: https://github.com/nspcc-dev/neo-go/blob/master/config/protocol.mainnet.neofs.yml https://github.com/nspcc-dev/neo-go/blob/master/config/protocol.testnet.neofs.yml

[AnnaShaleva]

More tests are added, need to update to master once #3190 is merged for successful test runs.

[AnnaShaleva]

Updated to fresh master, more tests added.

[AnnaShaleva]

@neo-project/core, a batch of tests is added, rebased onto fresh master and ready for review.

[vang1ong7ang]

me not in favor of this pr but me won't stop other people from liking it

however, at least, maintain it as a hardfork

same as #3175

[roman-khimov]

#3175 is HF-free to me (in that it can't be probed from contracts), but what I've said numerous times about these PRs (and the reason I wanted them in 3.7) is that we want to ensure C# node can process NeoFS chains. We've got two of them:

https://github.com/neo-project/neo/blob/master/src/Neo.CLI/config.fs.testnet.json https://github.com/neo-project/neo/blob/master/src/Neo.CLI/config.fs.mainnet.json

And they can't be processed unless Notary is available (from the genesis for testnet and at some height around a year ago for mainnet). That's why I specifically wanted to have this contract enabled at the genesis block. However, testnet likely can survive another HF enabled at 0 (like it has all of A/B/C at 0 now and it's not a problem). I have an idea about mainnet FS chain as well, luckily it switches to the notary at some height after the Aspidochelone HF, so maybe we can move B/C HF heights retrospectively and if D is to be compatible with FS chain (likely it'll be) then we could arrange safe (for FS chain) enablement of this contract at D. No guarantees, but we'll try to check it.

[AnnaShaleva]

however, at least, maintain it as a hardfork

Good, then let us check if NeoFS networks are compatible with hardfork-dependent Notary contract. I'll write the results here and update this PR to enable Notary contract starting from the D hardfork.