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 contract manifest build fixes #3704

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Native contract manifest build fixes #3704

merged 2 commits into from
Nov 26, 2024

Conversation

roman-khimov
Copy link
Member

Generic fixes from #3700.

These conditions are about filtering methods out. A method is excluded if its
ActiveFrom is strictly higher than the current HF, since if it's the same then
it should be present. Otherwise contract is broken at the height of this
particular HF.

Signed-off-by: Roman Khimov <[email protected]>
Technically, we can always buildHFSpecificMD() if contract is active, but we
just optimize the build out in some cases. switch slightly obfuscates this
and requires having the call in two branches.

Signed-off-by: Roman Khimov <[email protected]>
@roman-khimov roman-khimov added this to the v0.107.0 milestone Nov 25, 2024
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 83.04%. Comparing base (ff15e39) to head (9e7fd51).
Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
pkg/core/interop/context.go 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3704      +/-   ##
==========================================
- Coverage   83.04%   83.04%   -0.01%     
==========================================
  Files         335      335              
  Lines       46719    46729      +10     
==========================================
+ Hits        38800    38806       +6     
- Misses       6327     6332       +5     
+ Partials     1592     1591       -1     

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

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.

I'm OK with this fix, let me just check the Cockatrice states compatibility for the public networks. Haven't you check it?

@roman-khimov
Copy link
Member Author

Tests suggest that it works fine, we do invoke some C-methods at the C height in tests.

@AnnaShaleva
Copy link
Member

AnnaShaleva commented Nov 25, 2024

I'm more worried about proper native call scripts. Anyway, the check won't take long, my testnet node is already at 3M+ height.

@AnnaShaleva
Copy link
Member

Both testnet and mainnet are OK up to Domovoi+ height:

anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ go run ./scripts/compare-states/compare-states.go --ignore-height http://localhost:10332 http://seed1.neo.org:10332
at 0: 58a5157b7e99eeabf631291f1747ec8eb12ab89461cda888492b17a301de81e8 vs 58a5157b7e99eeabf631291f1747ec8eb12ab89461cda888492b17a301de81e8
at 5822486: e6328d1ee06d924233b73f278ce20c40c35a79fdc2c1f4fcc6f426b8f384432d vs e6328d1ee06d924233b73f278ce20c40c35a79fdc2c1f4fcc6f426b8f384432d
anna@kiwi:~/Documents/GitProjects/nspcc-dev/neo-go$ go run ./scripts/compare-states/compare-states.go --ignore-height http://localhost:20332 http://seed1t5.neo.org:20332
at 0: 62fd8ff9b0543aea352257db5b00bbb01d1bc0d2cc665e1f24cf5de0d16ebc7b vs 62fd8ff9b0543aea352257db5b00bbb01d1bc0d2cc665e1f24cf5de0d16ebc7b
at 5031329: 31af883ac28cd5030f0965bd61357b22985c995c1fbd3ed107f2e6e617dd8bb3 vs 31af883ac28cd5030f0965bd61357b22985c995c1fbd3ed107f2e6e617dd8bb3

@AnnaShaleva AnnaShaleva merged commit e89f9fe into master Nov 26, 2024
34 checks passed
@AnnaShaleva AnnaShaleva deleted the interop-md-fixes branch November 26, 2024 14:03
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.

2 participants