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

native: perform initialisation for the set of hardforks #3444

Merged
merged 1 commit into from
May 17, 2024

Conversation

AnnaShaleva
Copy link
Member

Close #3433.

Copy link

codecov bot commented May 17, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 86.13%. Comparing base (2d4993a) to head (c90f678).

Files Patch % Lines
pkg/core/native/management.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3444   +/-   ##
=======================================
  Coverage   86.12%   86.13%           
=======================================
  Files         331      331           
  Lines       38442    38448    +6     
=======================================
+ Hits        33110    33118    +8     
+ Misses       3804     3803    -1     
+ Partials     1528     1527    -1     

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

Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

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

Missing tests for it.

if err := native.Initialize(ic, activeIn, hfSpecificMD); err != nil {
return fmt.Errorf("initializing %s native contract at HF %d: %w", md.Name, activeIn, err)

// Deploy hardfok (contract's ActiveIn) is not a part of contract's active hardforks and
Copy link
Member

Choose a reason for hiding this comment

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

hardfork

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@AnnaShaleva
Copy link
Member Author

AnnaShaleva commented May 17, 2024

Missing tests for it.

I just don't know how to test it, we need a native contract to check that initialization works as expected, but we don't have such native contracts that are being initialized in non-ActiveIn hardfork.

Do you have any ideas? I thought about "fake" native contract that will be enabled on some config option as extension for testing.

Close #3433.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
@AnnaShaleva AnnaShaleva changed the title native: perform initialisation for the set of hardofkrs native: perform initialisation for the set of hardforks May 17, 2024
@roman-khimov roman-khimov merged commit 6be757a into master May 17, 2024
20 of 21 checks passed
@roman-khimov roman-khimov deleted the fix-native-init branch May 17, 2024 13:34
AnnaShaleva added a commit that referenced this pull request Jun 5, 2024
It doesn't work for contracts enabled starting from non-nil hardfork:
```
--- FAIL: TestStateroot_GetLatestStateHeight (0.00s)
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	initial gas supply is not set or wrong, setting default value	{"InitialGASSupply": "52000000"}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	mempool size is not set or wrong, setting default value	{"MemPoolSize": 50000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	P2PNotaryRequestPayloadPool size is not set or wrong, setting default value	{"P2PNotaryRequestPayloadPoolSize": 1000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxBlockSize is not set or wrong, setting default value	{"MaxBlockSize": 262144}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxBlockSystemFee is not set or wrong, setting default value	{"MaxBlockSystemFee": 900000000000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxTransactionsPerBlock is not set or wrong, using default value	{"MaxTransactionsPerBlock": 512}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxValidUntilBlockIncrement is not set or wrong, using default value	{"MaxValidUntilBlockIncrement": 86400}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	Hardforks are not set, using default value
    logger.go:146: 2024-06-04T17:08:35.266+0300	INFO	no storage version found! creating genesis block
    chain.go:227:
        	Error Trace:	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/neotest/chain/chain.go:227
        	            				/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/neotest/chain/chain.go:217
        	            				/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/services/stateroot/service_test.go:319
        	Error:      	Received unexpected error:
        	            	onPersist failed: VM has failed: at instruction 0 (SYSCALL): native contract descriptor cache is not initialized: contract c1e14f19c3e60d0b9244d06dd7ba9b113135ec3b, hardfork Default
        	Test:       	TestStateroot_GetLatestStateHeight
FAIL
coverage: 28.6% of statements

```

It happens because ActiveIn hardfork wasn't taken into account during
`latestHF` computation. This commit also removes the reusage of
`activeIn` variable in deploy procedure, it's misleading and not
necessary startign from #3444.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
AnnaShaleva added a commit that referenced this pull request Jul 15, 2024
It doesn't work for contracts enabled starting from non-nil hardfork:
```
--- FAIL: TestStateroot_GetLatestStateHeight (0.00s)
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	initial gas supply is not set or wrong, setting default value	{"InitialGASSupply": "52000000"}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	mempool size is not set or wrong, setting default value	{"MemPoolSize": 50000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	P2PNotaryRequestPayloadPool size is not set or wrong, setting default value	{"P2PNotaryRequestPayloadPoolSize": 1000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxBlockSize is not set or wrong, setting default value	{"MaxBlockSize": 262144}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxBlockSystemFee is not set or wrong, setting default value	{"MaxBlockSystemFee": 900000000000}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxTransactionsPerBlock is not set or wrong, using default value	{"MaxTransactionsPerBlock": 512}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	MaxValidUntilBlockIncrement is not set or wrong, using default value	{"MaxValidUntilBlockIncrement": 86400}
    logger.go:146: 2024-06-04T17:08:35.263+0300	INFO	Hardforks are not set, using default value
    logger.go:146: 2024-06-04T17:08:35.266+0300	INFO	no storage version found! creating genesis block
    chain.go:227:
        	Error Trace:	/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/neotest/chain/chain.go:227
        	            				/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/neotest/chain/chain.go:217
        	            				/home/anna/Documents/GitProjects/nspcc-dev/neo-go/pkg/services/stateroot/service_test.go:319
        	Error:      	Received unexpected error:
        	            	onPersist failed: VM has failed: at instruction 0 (SYSCALL): native contract descriptor cache is not initialized: contract c1e14f19c3e60d0b9244d06dd7ba9b113135ec3b, hardfork Default
        	Test:       	TestStateroot_GetLatestStateHeight
FAIL
coverage: 28.6% of statements

```

It happens because ActiveIn hardfork wasn't taken into account during
`latestHF` computation. This commit also removes the reusage of
`activeIn` variable in deploy procedure, it's misleading and not
necessary startign from #3444.

Signed-off-by: Anna Shaleva <shaleva.ann@nspcc.ru>
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.

Port natives initialization improvement
2 participants