Skip to content

*: move Notary contract out of P2PSigExtensions under Echidna hardfork #3478

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

Merged
merged 6 commits into from
Apr 30, 2025

Conversation

AnnaShaleva
Copy link
Member

@AnnaShaleva AnnaShaleva commented Jun 5, 2024

Close #3464.

@AnnaShaleva AnnaShaleva requested a review from roman-khimov June 5, 2024 15:46
@AnnaShaleva AnnaShaleva changed the title *: move Notary out of P2PSigExtensions under Domovoi hardfork *: move Notary contract out of P2PSigExtensions under Domovoi hardfork Jun 5, 2024
Copy link

codecov bot commented Jun 5, 2024

Codecov Report

Attention: Patch coverage is 64.86486% with 13 lines in your changes missing coverage. Please review.

Project coverage is 82.62%. Comparing base (9e8100a) to head (48f90d7).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
pkg/core/blockchain.go 14.28% 4 Missing and 2 partials ⚠️
pkg/core/native/notary.go 50.00% 3 Missing and 2 partials ⚠️
pkg/core/statesync/module.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3478      +/-   ##
==========================================
- Coverage   82.64%   82.62%   -0.03%     
==========================================
  Files         342      342              
  Lines       48277    48289      +12     
==========================================
- Hits        39899    39897       -2     
- Misses       6727     6748      +21     
+ Partials     1651     1644       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch from 551f9eb to 8871165 Compare July 15, 2024 10:53
@AnnaShaleva AnnaShaleva changed the title *: move Notary contract out of P2PSigExtensions under Domovoi hardfork *: move Notary contract out of P2PSigExtensions under Echidna hardfork Jul 15, 2024
@AnnaShaleva
Copy link
Member Author

Moved Notary functionality from Domovoi under Echidna hardfork.

@AnnaShaleva
Copy link
Member Author

Oh-oh, probably need to move it under next hardfork. Ref. #3554 and neo-project/neo#3454.

@roman-khimov
Copy link
Member

It still can be a part of Echidna. But for now we don't need to change anything anyway, it's totally C#-dependent.

@roman-khimov roman-khimov added the blocked Can't be done because of something label Nov 20, 2024
@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch 2 times, most recently from 69d8e62 to 95cfe4a Compare April 21, 2025 18:00
@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch from 3dd05c1 to d149f20 Compare April 30, 2025 10:24
New forks schedule is needed to be able to properly generate/restore basic chain
once Notary will be included in Echidna fork.

This commit also fixes a bug of statesync.Module when it's impossible to
initialize module after state sync is completed because of missing
MaxTraceableBlocks storage item if reffering by temporary storage
prefix:
```
    --- FAIL: TestStateSyncModule_Init/initialization_from_headers/blocks/mpt_synced_stages (0.01s)
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	initial gas supply is not set or wrong, setting default value	{"InitialGASSupply": "52000000"}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	P2PNotaryRequestPayloadPool size is not set or wrong, setting default value	{"P2PNotaryRequestPayloadPoolSize": 1000}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	MaxBlockSize is not set or wrong, setting default value	{"MaxBlockSize": 262144}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	MaxBlockSystemFee is not set or wrong, setting default value	{"MaxBlockSystemFee": 900000000000}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	Genesis MaxTraceableBlocks is not set or wrong, using default value	{"Genesis MaxTraceableBlocks": 3}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	MaxTransactionsPerBlock is not set or wrong, using default value	{"MaxTransactionsPerBlock": 512}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	MaxValidUntilBlockIncrement is not set or wrong, using default value	{"MaxValidUntilBlockIncrement": 5760}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	Genesis MaxValidUntilBlockIncrement is not set or wrong, using default value	{"Genesis MaxValidUntilBlockIncrement": 5760}
        logger.go:146: 2025-04-30T12:55:43.567+0300	INFO	GarbageCollectionPeriod is not set or wrong, using default value	{"GarbageCollectionPeriod": 10000}
        logger.go:146: 2025-04-30T12:55:43.569+0300	INFO	no storage version found! creating genesis block
        logger.go:146: 2025-04-30T12:55:43.570+0300	INFO	try to sync state for the latest state synchronisation point	{"point": 8, "evaluated chain's blockHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.570+0300	DEBUG	done processing headers	{"headerIndex": 1, "blockHeight": 0, "took": "458.95µs"}
        logger.go:146: 2025-04-30T12:55:43.570+0300	DEBUG	done processing headers	{"headerIndex": 2, "blockHeight": 0, "took": "438.832µs"}
        logger.go:146: 2025-04-30T12:55:43.571+0300	DEBUG	done processing headers	{"headerIndex": 3, "blockHeight": 0, "took": "430.826µs"}
        logger.go:146: 2025-04-30T12:55:43.571+0300	DEBUG	done processing headers	{"headerIndex": 4, "blockHeight": 0, "took": "455.433µs"}
        logger.go:146: 2025-04-30T12:55:43.572+0300	DEBUG	done processing headers	{"headerIndex": 5, "blockHeight": 0, "took": "266.015µs"}
        logger.go:146: 2025-04-30T12:55:43.572+0300	DEBUG	done processing headers	{"headerIndex": 6, "blockHeight": 0, "took": "266.615µs"}
        logger.go:146: 2025-04-30T12:55:43.572+0300	DEBUG	done processing headers	{"headerIndex": 7, "blockHeight": 0, "took": "409.386µs"}
        logger.go:146: 2025-04-30T12:55:43.573+0300	DEBUG	done processing headers	{"headerIndex": 8, "blockHeight": 0, "took": "397.653µs"}
        logger.go:146: 2025-04-30T12:55:43.573+0300	DEBUG	done processing headers	{"headerIndex": 9, "blockHeight": 0, "took": "390.179µs"}
        logger.go:146: 2025-04-30T12:55:43.573+0300	INFO	headers are in sync	{"headerHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.573+0300	INFO	MPT billet initialized	{"height": 8, "state root": "c3d0e2d2905577864b43abdf995bd9c95084036983020b7c80b26786f35341fc"}
        logger.go:146: 2025-04-30T12:55:43.573+0300	INFO	try to sync state for the latest state synchronisation point	{"point": 8, "evaluated chain's blockHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.573+0300	INFO	headers are in sync	{"headerHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.573+0300	INFO	MPT billet initialized	{"height": 8, "state root": "c3d0e2d2905577864b43abdf995bd9c95084036983020b7c80b26786f35341fc"}
        logger.go:146: 2025-04-30T12:55:43.574+0300	INFO	try to sync state for the latest state synchronisation point	{"point": 8, "evaluated chain's blockHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.574+0300	INFO	headers are in sync	{"headerHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.574+0300	INFO	MPT billet initialized	{"height": 8, "state root": "c3d0e2d2905577864b43abdf995bd9c95084036983020b7c80b26786f35341fc"}
        logger.go:146: 2025-04-30T12:55:43.576+0300	INFO	MPT is in sync	{"height": 8}
        logger.go:146: 2025-04-30T12:55:43.576+0300	INFO	try to sync state for the latest state synchronisation point	{"point": 8, "evaluated chain's blockHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.576+0300	INFO	headers are in sync	{"headerHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.576+0300	INFO	MPT billet initialized	{"height": 8, "state root": "c3d0e2d2905577864b43abdf995bd9c95084036983020b7c80b26786f35341fc"}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	MPT is in sync	{"stateroot height": 8}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	blocks are in sync	{"blockHeight": 8}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	state is in sync	{"state sync point": 8}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	jumping to state sync point	{"state sync point": 8}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	try to sync state for the latest state synchronisation point	{"point": 8, "evaluated chain's blockHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	headers are in sync	{"headerHeight": 9}
        logger.go:146: 2025-04-30T12:55:43.577+0300	INFO	MPT billet initialized	{"height": 8, "state root": "c3d0e2d2905577864b43abdf995bd9c95084036983020b7c80b26786f35341fc"}
        logger.go:146: 2025-04-30T12:55:43.578+0300	INFO	MPT is in sync	{"stateroot height": 8}
        logger.go:146: 2025-04-30T12:55:43.578+0300	INFO	persisted to disk	{"blocks": 0, "keys": 225, "headerHeight": 9, "blockHeight": 8, "took": "54.213µs"}
    logger.go:146: 2025-04-30T12:55:43.578+0300	INFO	persisted to disk	{"blocks": 9, "keys": 278, "headerHeight": 9, "blockHeight": 9, "took": "79.611µs"}
panic: failed to retrieve MaxTraceableBlock storage item from Policy contract storage by key 70f9ffffff17 at height 8: key not found [recovered]
	panic: failed to retrieve MaxTraceableBlock storage item from Policy contract storage by key 70f9ffffff17 at height 8: key not found

goroutine 195 [running]:
testing.tRunner.func1.2({0xdce600, 0xc0000ad3c0})
	/home/anna/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
	/home/anna/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1635 +0x35e
panic({0xdce600?, 0xc0000ad3c0?})
	/home/anna/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/runtime/panic.go:785 +0x132
github.com/nspcc-dev/neo-go/pkg/core/statesync.(*Module).getLatestSavedBlock(0xc0001ec8c0, 0x8)
	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/statesync/module.go:314 +0x437
github.com/nspcc-dev/neo-go/pkg/core/statesync.(*Module).defineSyncStage(0xc0001ec8c0)
	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/statesync/module.go:279 +0x9fe
github.com/nspcc-dev/neo-go/pkg/core/statesync.(*Module).Init(0xc0001ec8c0, 0x9)
	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/statesync/module.go:190 +0x465
github.com/nspcc-dev/neo-go/pkg/core/statesync_test.TestStateSyncModule_Init.func9(0xc00018e340)
	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/core/statesync/neotest_test.go:237 +0x1265
testing.tRunner(0xc00018e340, 0xc00028bc68)
	/home/anna/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 64
	/home/anna/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.linux-amd64/src/testing/testing.go:1743 +0x390

```

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch from d149f20 to b0f6620 Compare April 30, 2025 10:31
@AnnaShaleva AnnaShaleva removed the blocked Can't be done because of something label Apr 30, 2025
@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch 3 times, most recently from 65bb150 to 1274321 Compare April 30, 2025 11:02
@AnnaShaleva AnnaShaleva marked this pull request as ready for review April 30, 2025 11:02
@AnnaShaleva
Copy link
Member Author

@roman-khimov, I can't request review from copilot, do I need some extra rights for that?

@roman-khimov roman-khimov requested a review from Copilot April 30, 2025 15:00
@roman-khimov
Copy link
Member

I think it needs to be enabled for your account (check GH settings)

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request decouples the Notary contract deployment from the P2PSigExtensions flag and instead gates its activation through hardfork configurations (including Echidna and Domovoi). Key changes include:

  • Removing the conditional block based on P2PSigExtensions in contract creation.
  • Updating numerous tests to eliminate dependencies on P2PSigExtensions and to use hardfork-defined activation.
  • Adjusting configuration and documentation to reflect the new activation thresholds for native contracts.

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/core/native/native_test/neo_test.go Configured hardfork mapping for Notary contract activation in tests.
pkg/core/native/native_test/management_test.go Updated expected events and removed P2PSigExtensions-related settings.
pkg/core/native/gas_test.go Removed forced P2PSigExtensions enabling in custom config for GAS tests.
pkg/core/native/contract.go Removed the condition to add the Notary contract only if P2PSigExtensions is set.
config/protocol.unit_testnet.yml Updated hardfork activation heights to reflect new protocol upgrade logic.
docs/node-configuration.md Adjusted Notary documentation to remove reference to P2PSigExtensions.
[other test and internal files] Similar removals of P2PSigExtensions settings to be replaced by hardfork checks.
Comments suppressed due to low confidence (4)

pkg/core/native/contract.go:101

  • The removal of the cfg.P2PSigExtensions conditional means the Notary contract is now always deployed. Verify that downstream modules and tests relying on conditional deployment are updated to use hardfork configuration exclusively.
if cfg.P2PSigExtensions {

pkg/core/native/native_test/management_test.go:49

  • The tests have been updated to remove P2PSigExtensions enabling and instead rely on hardfork mappings for Notary activation. Ensure all test scenarios covering Notary deployment via hardfork configuration are adequately exercised.
cfg.P2PSigExtensions = true

config/protocol.unit_testnet.yml:20

  • The hardfork activation heights have been updated. Confirm that these new values are consistent with the intended network upgrade schedule and documented across all relevant channels.
Aspidochelone: 1, Basilisk: 2, Cockatrice: 3, Domovoi: 4, Echidna: 5

docs/node-configuration.md:417

  • The Notary contract deployment no longer relies on the P2PSigExtensions flag; update the documentation to reflect that its activation is now governed by hardfork configuration.
| P2PSigExtensions | `bool` | `false` | ... |

@roman-khimov
Copy link
Member

--- FAIL: TestDBRestoreDump (0.10s)
--- PASS: TestDBRestoreDump/excessive_restore_parameters (0.00s)
panic: item with id = -10 and key = 0a is not initialized [recovered]
panic: item with id = -10 and key = 0a is not initialized

Close #3464. Adjust tests, adjust basic chain hardforks schedule because
we need Notary contract to be enabled startign from block 7 at basic
chain.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Use `nativehashes.Notary instead of
(*Blockchain).GetNotaryContractScriptHash.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
Ref. neo-project/neo#3178.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
There should be no contract updates in Domovoi.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva force-pushed the hf-dependant-notary branch from 0aaa5ce to 48f90d7 Compare April 30, 2025 16:31
@AnnaShaleva
Copy link
Member Author

--- FAIL: TestDBRestoreDump (0.10s)
--- PASS: TestDBRestoreDump/excessive_restore_parameters (0.00s)
panic: item with id = -10 and key = 0a is not initialized [recovered]
panic: item with id = -10 and key = 0a is not initialized

It's due to #3478 (comment). Unexpected. I reverted it and renamed activeIn to notaryActiveIn, let it be the global variable.

@roman-khimov roman-khimov merged commit bf0ae1b into master Apr 30, 2025
32 of 34 checks passed
@roman-khimov roman-khimov deleted the hf-dependant-notary branch April 30, 2025 17:08
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 this pull request may close these issues.

Move Notary contract under E hardfork
2 participants