Implement NotaryAssisted transaction attribute #3175

AnnaShaleva · 2024-03-06T09:00:44Z

Description

Close #2896. Use a stub for native Notary contract hash since this contract is not implemented yet. Thus, technically, NotaryAssisted attribute verification will always fail on real network until native Notary is implemented.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

A group of unit tests for NotaryAssisted attribute.

Checklist:

Close neo-project#2896. Use a stub for native Notary contract hash since this contract is not implemented yet. Thus, technically, NotaryAssisted attribute verification will always fail on real network until native Notary is implemented. Signed-off-by: Anna Shaleva <[email protected]>

Port neo-project/neo#3175. Ref. neo-project/neo#2896. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva · 2024-03-06T12:08:22Z

I'm not allowed to request review, set labels or resolve conversations. Maybe someone from neo-project with access rights can allow me at least to request review?

shargon · 2024-03-06T13:26:12Z

I'm not allowed to request review, set labels or resolve conversations. Maybe someone from neo-project with access rights can allow me at least to request review?

You didn't accept your role xD, I sent you an invitation time ago... Time to review your email? 😄

src/Neo/SmartContract/Native/PolicyContract.cs

Jim8y · 2024-03-06T13:35:55Z

Hi @AnnaShaleva , we are trying to add as much comments in the core as possible, may you please add comments to explain CalculateNetworkFee.

src/Neo/SmartContract/Native/PolicyContract.cs

…ribute Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva · 2024-03-06T15:12:08Z

may you please add comments to explain CalculateNetworkFee.

Sure, fixed.

roman-khimov

GAS.OnPersist change is missing for the attribute.

src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs

src/Neo/Network/P2P/Payloads/NotaryAssisted.cs

Transactions network fee should be split between Primary node and Notary nodes. Signed-off-by: Anna Shaleva <[email protected]>

Signed-off-by: Anna Shaleva <[email protected]>

Once Notary contract is implemented, this hash will be replaced by a proper Notary contract hash. The exact value won't be changed since Notary contract has constant hash as any other native contract. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva · 2024-03-07T09:40:52Z

GAS.OnPersist change is missing for the attribute.

Completely forgot about it, fixed.

vncoelho · 2024-03-07T09:42:10Z

Maybe this PR we merge after next release.

AnnaShaleva · 2024-03-07T09:45:17Z

Maybe this PR we merge after next release.

This PR doesn't hurt because currently in real network NotaryAssisted attribute is always invalid since transaction must have Notary contract as a signer, and Notary contract is not implemented yet.

Thus, I vote to merge it before 3.7. What do you think?

roman-khimov · 2024-03-07T10:13:25Z

It goes step by step, @vncoelho. What this delay would buy us? The attribute logic is rather simple on its own.

shargon

I need more time to review the notary proposal, honestly, I didn't expect it to be merged in 3.7 and it is already beginning to produce changes in the core

AnnaShaleva · 2025-03-03T08:10:14Z

@Jim8y @vncoelho @cschuchardt88 need your review here, we've agreed to move on with this PR, and we need it to be a part of 3.8.

cschuchardt88 · 2025-03-03T20:19:22Z

@AnnaShaleva You have errors that need fixing 1st.

Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva · 2025-03-04T07:57:36Z

You have errors that need fixing 1st.

Thanks for pointing that out, it was due to recent testing framework upgrade happened on master. Unit-test is fixed (see the latest commit), nothing is changed in the core logic, but we need to collect approvals from @shargon @nan01ab and @roman-khimov one more time.

Problem: `getAttributeFee` method of native Policy contract has two implementations: pre-Echidna and post-Echidna (ref. #3175). The problem is that two implementaitons of this method have reversed ActiveIn/ActiveTill meaning. Solution: GetAttributeFeeV0 implementaiton of Policy's `getAttributeFee` method should be enabled *till* Echidna, whereas GetAttributeFee should be enabled *starting from* Echidna. Signed-off-by: Anna Shaleva <[email protected]>

Problem: `[get/set]AttributeFee` method of native Policy contract has two implementations: pre-Echidna and post-Echidna (ref. #3175). The problem is that two implementaitons of this method have reversed ActiveIn/ActiveTill meaning. Solution: GetAttributeFeeV0 implementaiton of Policy's `getAttributeFee` method should be enabled *till* Echidna, whereas GetAttributeFee should be enabled *starting from* Echidna. The same is relevant for SetAttributeFeeV0 and SetAttributeFeeV1 implementations. Signed-off-by: Anna Shaleva <[email protected]>

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

* Native: swap Policy's `[get/set]AttributeFee` implementations Problem: `[get/set]AttributeFee` method of native Policy contract has two implementations: pre-Echidna and post-Echidna (ref. #3175). The problem is that two implementaitons of this method have reversed ActiveIn/ActiveTill meaning. Solution: GetAttributeFeeV0 implementaiton of Policy's `getAttributeFee` method should be enabled *till* Echidna, whereas GetAttributeFee should be enabled *starting from* Echidna. The same is relevant for SetAttributeFeeV0 and SetAttributeFeeV1 implementations. Signed-off-by: Anna Shaleva <[email protected]> * Native: rename Policy's GetAttributeFee to GetAttributeFeeV1 This renaming allows to avoid bugs and collisions with ActiveIn/ActiveTill. Also, this renaming follows the setters style (for setters we have `SetAttributeFeeV0` and `SetAttributeFeeV1`. No functional changes, just a refactoring. Contract manifest is not affected. Signed-off-by: Anna Shaleva <[email protected]> --------- Signed-off-by: Anna Shaleva <[email protected]> Co-authored-by: Shargon <[email protected]> Co-authored-by: NGD Admin <[email protected]>

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

* Implement NotaryAssisted transaction attribute Close #2896. Use a stub for native Notary contract hash since this contract is not implemented yet. Thus, technically, NotaryAssisted attribute verification will always fail on real network until native Notary is implemented. Signed-off-by: Anna Shaleva <[email protected]> * Payloads: add doc to CalculateNetworkFee method of NotaryAssisted attribute Signed-off-by: Anna Shaleva <[email protected]> * Native: add NotaryAssisted attributes handler to Gas OnPersist Transactions network fee should be split between Primary node and Notary nodes. Signed-off-by: Anna Shaleva <[email protected]> * Payloads: adjust comment to NotaryAssisted attribute Signed-off-by: Anna Shaleva <[email protected]> * Payloads: temporary use hard-coded Notary contract hash Once Notary contract is implemented, this hash will be replaced by a proper Notary contract hash. The exact value won't be changed since Notary contract has constant hash as any other native contract. Signed-off-by: Anna Shaleva <[email protected]> * Payloads: replace hard-coded Notary hash value with calculated one No functional changes, just a refactoring for better code readability. Signed-off-by: Anna Shaleva <[email protected]> * Native: implement native Notary contract Close #2897. Depends on #3175. Signed-off-by: Anna Shaleva <[email protected]> * Native: fix typo in the exception message Signed-off-by: Anna Shaleva <[email protected]> * Native: use more syntactic sugar No functional changes, just a refactoring. Signed-off-by: Anna Shaleva <[email protected]> * Notary: add unit tests for OnNEP17Payment and ExpirationOf methods Also, mark improper code of Withdraw method with TODO, it should be reworked. Signed-off-by: Anna Shaleva <[email protected]> * Fix transfer * Update tests/Neo.UnitTests/SmartContract/Native/UT_Notary.cs * Update UT_Notary.cs * fix * Fix notary * Fix notary * Notary: add unit tests for LockDepositUntil and BalanceOf methods Also, format code. Signed-off-by: Anna Shaleva <[email protected]> * Notary: test GAS distribution with FeePerKey update Test the situation described in https://github.com/neo-project/neo/pull/3175/files/00b54ff6d20cc84b435beaa790fe72a9d8f78bec#r1530493475. Signed-off-by: Anna Shaleva <[email protected]> * Notary: add test for Withdraw Signed-off-by: Anna Shaleva <[email protected]> * Notary: remove unused code No functional changes, I just finally made my code analizer work properly. Signed-off-by: Anna Shaleva <[email protected]> * Attributtes: fix NotaryAssisted attribute documentation format Co-authored-by: Shargon <[email protected]> * Native: update to the fresh master Fetch changes from the fresh master and fix build errors. Signed-off-by: Anna Shaleva <[email protected]> * Notary: sync up with master changes No functional changes, just adopt latest changes in syntax from master to make the code buildable and make unit tests pass. Signed-off-by: Anna Shaleva <[email protected]> * Notary: make the contract active starting from Echidna. Signed-off-by: Anna Shaleva <[email protected]> * Notary: add NEP-27 to the list of supported standards Port nspcc-dev/neo-go#3792. Signed-off-by: Anna Shaleva <[email protected]> * Notary: adjust deposit expiration rule Follow nspcc-dev/neo-go#3211. Signed-off-by: Anna Shaleva <[email protected]> * clean usings * Update tests/Neo.UnitTests/Extensions/Nep17NativeContractExtensions.cs Co-authored-by: Will <[email protected]> * Notary: improve long line format No functional changes. Signed-off-by: Anna Shaleva <[email protected]> * Notary: prettify exception thrown on unexpected OnNEP17Payment data Throw FormatException instead of InvalidCastException on invalid data passed to OnNEP17Payment. Signed-off-by: Anna Shaleva <[email protected]> * Notary: don't FAULT execution in case of non-tx verification container Return `false` in case if execution container for `verify` is not a transaction. Signed-off-by: Anna Shaleva <[email protected]> * Notary: unify deposit lock validity check No functional changes, just a refactoring. Signed-off-by: Anna Shaleva <[email protected]> * Notary: extend `verify` with signature argument validation Ref. https://github.com/neo-project/neo/pull/3178/files/56acd97e4b2e93601b7e3d4b15b5ccafc9455fcd#r2010316964. Signed-off-by: Anna Shaleva <[email protected]> * Notary: add a note about Deposit serialization No functional changes. Signed-off-by: Anna Shaleva <[email protected]> * UnitTests: remove NotaryAssisted fees unit test This test requires Notary nodes to be properly designated via native RoleManagement contract, it's hard to mock this behaviour during test, I wasn't able to properly do it. This test may be restored once we have better testing system. Signed-off-by: Anna Shaleva <[email protected]> * Notary: replace exceptions with false return for `withdraw` Ref. #3178 (comment). Signed-off-by: Anna Shaleva <[email protected]> * Notary: refactor variable definition No functional changes, ref. #3178 (comment). Signed-off-by: Anna Shaleva <[email protected]> * Notary: migrate to Policy.GetAttributeFeeV1 usage No functional changes, just adopt the #3859. Signed-off-by: Anna Shaleva <[email protected]> * Review Notary (#3870) * Update Notary.cs * Update Notary.cs * Update Notary.cs * fix * clean * Clean * Update src/Neo/SmartContract/Native/Notary.cs * Update tests/Neo.UnitTests/TestUtils.Transaction.cs Co-authored-by: Christopher Schuchardt <[email protected]> * Update tests/Neo.UnitTests/TestUtils.Transaction.cs Co-authored-by: Christopher Schuchardt <[email protected]> * Update tests/Neo.UnitTests/TestUtils.Transaction.cs * Update tests/Neo.UnitTests/TestUtils.Transaction.cs * Notary: adapt Policy-based MaxValidUntilBlockIncrement Fetch recent update from #3861. Signed-off-by: Anna Shaleva <[email protected]> * Notary: rename manifest parameters to follow the native style Unify parameters naming to follow the style of other native contracts. No functional changes. Signed-off-by: Anna Shaleva <[email protected]> * Notary: adjust required callflags for some methods Signed-off-by: Anna Shaleva <[email protected]> * Notary: fix Notary node scripthash construction Notary node account is always a standard single signature script. This commit fixes state difference between Go and C# nodes at block 9762 of NeoFS Testnet: ``` go run scripts/compare-dumps/compare-dumps.go ./godump-echidna-neofs-testnet/ ../../neo-project/neo/neo-cli-notary-testnet/Storage_2bdb2b5f/ Processing directory BlockStorage_0 Processing directory BlockStorage_100000 file BlockStorage_100000/dump-block-10000.json: block 9762, changes length mismatch: 13 vs 20 compare-dumps dumpDirA dumpDirB exit status 1 ``` The reason is in improper construction of Notary node address which leads to the fact that notary GAS reward goes to wrong addresses. Here's a part of OnPersist application log for this block generated by C# node that contains wrong GAS receiver addresses: ``` { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "Z0B9UyyyUsDlW4OgICWJ+nwLyZc=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "5Sk5i/oPh7IBb7VvlHfiUjX2LHU=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "HcOwwNv9mwY1i3SF4d5qX8jDdJs=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "8a2TEhIZFJ/lZDklkMs+d6i7OWI=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "Oke4DvOe+XZ+3mQnZ5nE1Ilm3jY=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "7frkdtIbPRN2ijaHnw+tfUxMzA8=" }, { "type": "Integer", "value": "11428571" } ] } }, { "contract": "0xd2a4cff31913016155e38e474a2c06d08be276cf", "eventname": "Transfer", "state": { "type": "Array", "value": [ { "type": "Any" }, { "type": "ByteString", "value": "YjI6lI1pOy/OxH1PjcG/yIXwD/4=" }, { "type": "Integer", "value": "11428571" } ] } } ], "stack": [ ], "trigger": "OnPersist", "vmstate": "HALT" } ``` Signed-off-by: Anna Shaleva <[email protected]> --------- Signed-off-by: Anna Shaleva <[email protected]> Co-authored-by: Jimmy <[email protected]> Co-authored-by: Fernando Diaz Toledano <[email protected]> Co-authored-by: Christopher Schuchardt <[email protected]> Co-authored-by: Will <[email protected]> Co-authored-by: NGD Admin <[email protected]> Co-authored-by: Vitor Nazário Coelho <[email protected]>

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva added a commit to AnnaShaleva/neo-devpack-dotnet that referenced this pull request Mar 6, 2024

Add NotaryAssisted transaction attribute type

24c6641

Port neo-project/neo#3175. Ref. neo-project/neo#2896. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva mentioned this pull request Mar 6, 2024

Add NotaryAssisted transaction attribute type neo-project/neo-devpack-dotnet#983

Merged

AnnaShaleva added a commit to AnnaShaleva/neo-modules that referenced this pull request Mar 6, 2024

Add NotaryAssisted transaction attribute type

8f55314

Port neo-project/neo#3175. Ref. neo-project/neo#2896. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva mentioned this pull request Mar 6, 2024

Add NotaryAssisted transaction attribute type neo-project/neo-modules#884

Closed

roman-khimov requested review from shargon, Jim8y and roman-khimov March 6, 2024 12:34

shargon reviewed Mar 6, 2024

View reviewed changes

src/Neo/SmartContract/Native/PolicyContract.cs Outdated Show resolved Hide resolved

Jim8y reviewed Mar 6, 2024

View reviewed changes

src/Neo/SmartContract/Native/PolicyContract.cs Show resolved Hide resolved

Payloads: add doc to CalculateNetworkFee method of NotaryAssisted att…

acec1b0

…ribute Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva requested review from shargon and Jim8y March 6, 2024 15:12

roman-khimov requested changes Mar 6, 2024

View reviewed changes

src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs Outdated Show resolved Hide resolved

roman-khimov reviewed Mar 7, 2024

View reviewed changes

src/Neo/Network/P2P/Payloads/NotaryAssisted.cs Outdated Show resolved Hide resolved

AnnaShaleva added 3 commits March 7, 2024 12:27

Native: add NotaryAssisted attributes handler to Gas OnPersist

1508d4f

Transactions network fee should be split between Primary node and Notary nodes. Signed-off-by: Anna Shaleva <[email protected]>

Payloads: adjust comment to NotaryAssisted attribute

8b547c9

Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva requested a review from roman-khimov March 7, 2024 09:41

roman-khimov previously approved these changes Mar 7, 2024

View reviewed changes

shargon reviewed Mar 8, 2024

View reviewed changes

Merge branch 'master' into notary-assisted

00b54ff

roman-khimov mentioned this pull request Feb 27, 2025

[Feb. 27] Core Dev Regular Meeting Proposal Discussion #3793

Closed

Merge branch 'master' into notary-assisted

ee4232a

roman-khimov previously approved these changes Mar 3, 2025

View reviewed changes

Jim8y added the Ready to Merge label Mar 4, 2025

UnitTests: migrate NotaryAssisted test from MSTestV1 to MSTestV2

803d872

Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva dismissed stale reviews from roman-khimov and shargon via 803d872 March 4, 2025 07:50

AnnaShaleva requested review from shargon, roman-khimov and nan01ab March 4, 2025 07:58

roman-khimov approved these changes Mar 4, 2025

View reviewed changes

shargon approved these changes Mar 4, 2025

View reviewed changes

NGDAdmin merged commit 4c7bae4 into neo-project:master Mar 4, 2025
7 checks passed

AnnaShaleva mentioned this pull request Mar 31, 2025

Native: swap Policy's [get/set]AttributeFee implementations #3859

Merged

9 tasks

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 1, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

f276bd5

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva mentioned this pull request Apr 1, 2025

core: move NotaryAssisted under Echidna hardfork nspcc-dev/neo-go#3854

Merged

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 1, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

b550289

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 1, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

0940198

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 21, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

40e66dd

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

mialbu mentioned this pull request Apr 28, 2025

[Notary] Add NotaryAssisted transaction attribute neow3j/neow3j#1108

Open

mialbu mentioned this pull request Apr 29, 2025

Support new native Notary contract neow3j/neow3j#1114

Open

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 29, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

0a140f3

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

AnnaShaleva added a commit to nspcc-dev/neo-go that referenced this pull request Apr 29, 2025

core: move NotaryAssisted attribute out of P2PSigExtensions

57fd79a

Move it under Echidna hardfork. Port neo-project/neo#3175. Signed-off-by: Anna Shaleva <[email protected]>

Implement NotaryAssisted transaction attribute #3175

Implement NotaryAssisted transaction attribute #3175

Uh oh!

Conversation

AnnaShaleva commented Mar 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Checklist:

Uh oh!

AnnaShaleva commented Mar 6, 2024

Uh oh!

shargon commented Mar 6, 2024

Uh oh!

Uh oh!

Jim8y commented Mar 6, 2024

Uh oh!

Uh oh!

AnnaShaleva commented Mar 6, 2024

Uh oh!

roman-khimov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AnnaShaleva commented Mar 7, 2024

Uh oh!

vncoelho commented Mar 7, 2024

Uh oh!

AnnaShaleva commented Mar 7, 2024

Uh oh!

roman-khimov commented Mar 7, 2024

Uh oh!

shargon left a comment

Choose a reason for hiding this comment

Uh oh!

AnnaShaleva commented Mar 3, 2025

Uh oh!

cschuchardt88 commented Mar 3, 2025

Uh oh!

AnnaShaleva commented Mar 4, 2025

Uh oh!

Uh oh!

Uh oh!

AnnaShaleva commented Mar 6, 2024 •

edited

Loading