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

Perform native contract initialization for the set of hardforks #3202

Merged
merged 19 commits into from
May 10, 2024

Conversation

shargon
Copy link
Member

@shargon shargon commented Apr 23, 2024

Close #3200

vncoelho
vncoelho previously approved these changes Apr 23, 2024
Copy link
Member

@vncoelho vncoelho left a comment

Choose a reason for hiding this comment

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

Looks good to me, @shargon

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

The logic looks good now, let me test it on mainnet and custom privnet.

src/Neo/SmartContract/Native/NativeContract.cs Outdated Show resolved Hide resolved
@AnnaShaleva
Copy link
Member

Something has happened with the Neo unit-tests, they are hanging (checked this locally, the result is the same):

  Neo.GUI -> /home/anna/Documents/GitProjects/neo-project/neo/src/Neo.GUI/bin/Debug/net8.0-windows/Neo.GUI.dll
  Neo.UnitTests -> /home/anna/Documents/GitProjects/neo-project/neo/tests/Neo.UnitTests/bin/Debug/net8.0/Neo.UnitTests.dll
Test run for /home/anna/Documents/GitProjects/neo-project/neo/tests/Neo.UnitTests/bin/Debug/net8.0/Neo.UnitTests.dll (.NETCoreApp,Version=v8.0)
Microsoft (R) Test Execution Command Line Tool Version 17.9.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

// If is not configured, the Genesis is the a initialized block
if (index == 0 && ActiveIn is null)
{
hardforks = hfs.ToArray();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
hardforks = hfs.ToArray();
hardforks = [];

Plus move this above the if statement above it; line:90.

src/Neo/SmartContract/Native/ContractManagement.cs Outdated Show resolved Hide resolved
@shargon
Copy link
Member Author

shargon commented Apr 26, 2024

Something has happened with the Neo unit-tests, they are hanging (checked this locally, the result is the same):

  Neo.GUI -> /home/anna/Documents/GitProjects/neo-project/neo/src/Neo.GUI/bin/Debug/net8.0-windows/Neo.GUI.dll
  Neo.UnitTests -> /home/anna/Documents/GitProjects/neo-project/neo/tests/Neo.UnitTests/bin/Debug/net8.0/Neo.UnitTests.dll
Test run for /home/anna/Documents/GitProjects/neo-project/neo/tests/Neo.UnitTests/bin/Debug/net8.0/Neo.UnitTests.dll (.NETCoreApp,Version=v8.0)
Microsoft (R) Test Execution Command Line Tool Version 17.9.0 (x64)
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.

looks like Neo receive all the hardforks, but it need null :S

@cschuchardt88
Copy link
Member

cschuchardt88 commented Apr 26, 2024

@shargon @AnnaShaleva is it just for this PR? The test handing issue.

@shargon
Copy link
Member Author

shargon commented Apr 26, 2024

@shargon @AnnaShaleva is it just for this PR? The test handing issue.

yes, but it's because without null, neo contract is not initialized in out tests

Copy link
Member

@AnnaShaleva AnnaShaleva left a comment

Choose a reason for hiding this comment

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

With these suggestions this code works as expected even in case of multiple hardforks being enabled at the same height. Also, with these suggestions unit tests are passed and genesis state matches the expected one (I've checked both genesis storage changes and notifications against NeoGo).

src/Neo/SmartContract/Native/ContractManagement.cs Outdated Show resolved Hide resolved
src/Neo/SmartContract/Native/ContractManagement.cs Outdated Show resolved Hide resolved
src/Neo/SmartContract/Native/ContractManagement.cs Outdated Show resolved Hide resolved
According to new logic, an empty set of hard-forks will be returned in
case of genesis initialization with all hardforks disabled.

Ref.
#3202 (comment).

Signed-off-by: Anna Shaleva <[email protected]>
@AnnaShaleva
Copy link
Member

@shargon, I've fixed the test, please, see the last commit. Now all tests are passing and the logic works as expected. Let's review this PR one more time (if needed) and merge, we need to include it into 3.7.

@vncoelho
Copy link
Member

vncoelho commented May 6, 2024

I will test here as well.

@NGDAdmin NGDAdmin merged commit 1e6404b into master May 10, 2024
6 checks passed
@NGDAdmin NGDAdmin deleted the core-close-3200 branch May 10, 2024 02:41
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.

Perform native contract initialization for the set of hardforks
6 participants