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.

Sign up for GitHub

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

Jump to bottom

native: add getCommitteeAddress method #3362

Merged
merged 3 commits into from
Mar 22, 2024
Merged

native: add getCommitteeAddress method #3362

merged 3 commits into from
Mar 22, 2024

Conversation

AliceInHunterland
Copy link
Contributor

@AliceInHunterland AliceInHunterland commented Mar 19, 2024 •
edited
Loading

@AnnaShaleva
Copy link
Member

It should be ported to the corresponding interop package and to the Neo-specific invoker.

AnnaShaleva
AnnaShaleva reviewed Mar 19, 2024
View reviewed changes
pkg/core/blockchain.go Outdated Show resolved Hide resolved
pkg/core/native/native_neo.go Outdated Show resolved Hide resolved
pkg/core/native/native_neo.go Outdated Show resolved Hide resolved
pkg/interop/native/neo/neo.go Show resolved Hide resolved
pkg/core/native/native_test/neo_test.go Outdated Show resolved Hide resolved
AnnaShaleva
AnnaShaleva reviewed Mar 21, 2024
View reviewed changes
pkg/core/native/native_test/neo_test.go Outdated Show resolved Hide resolved
pkg/core/native/native_test/neo_test.go Show resolved Hide resolved
pkg/rpcclient/neo/neo.go Outdated Show resolved Hide resolved
pkg/rpcclient/neo/neo.go Outdated Show resolved Hide resolved
examples/engine/go.mod Outdated Show resolved Hide resolved
@AnnaShaleva
Copy link
Member

Tests are failing.

AnnaShaleva
AnnaShaleva reviewed Mar 22, 2024
View reviewed changes
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.

Tests are still failing.

pkg/core/native/native_test/neo_test.go Show resolved Hide resolved
@AnnaShaleva
Copy link
Member

AnnaShaleva commented Mar 22, 2024 •
edited
Loading

Block 20 stateroot from RPC tests needs to be updated because you change native Management's state by adding new method to native Neo's manifest.

@codecov Codecov
Copy link

codecov bot commented Mar 22, 2024 •
edited
Loading

Codecov Report

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

Project coverage is 84.80%. Comparing base (900ef46) to head (f8ebed2).
Report is 2 commits behind head on master.

Files Patch % Lines
pkg/rpcclient/neo/neo.go 0.00% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3362      +/-   ##
==========================================
- Coverage   84.81%   84.80%   -0.02%     
==========================================
  Files         331      331              
  Lines       44961    44969       +8     
==========================================
  Hits        38135    38135              
- Misses       5314     5321       +7     
- Partials     1512     1513       +1

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

AnnaShaleva
AnnaShaleva reviewed Mar 22, 2024
View reviewed changes
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.

Otherwise LGTM!

internal/fakechain/fakechain.go Outdated Show resolved Hide resolved
@AliceInHunterland
*: update interop deps
f8ebed2 
Signed-off-by: Ekaterina Pavlova <[email protected]>
@AnnaShaleva AnnaShaleva merged commit fa1c07e into master Mar 22, 2024
16 of 20 checks passed
@AnnaShaleva AnnaShaleva deleted the getCommitteeAddress branch March 22, 2024 14:13
roman-khimov
roman-khimov reviewed Mar 22, 2024
View reviewed changes
pkg/core/native/native_neo.go
@@ -232,6 +232,10 @@ func newNEO(cfg config.ProtocolConfiguration) *NEO {
md = newMethodAndPrice(n.getCommittee, 1<<16, callflag.ReadStates)
n.AddMethod(md, desc)

desc = newDescriptor("getCommitteeAddress", smartcontract.Hash160Type)
Copy link
Member

Choose a reason for hiding this comment

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

It's only available after the Cockatrice hardfork, so at the very minimum this needs to be updated in #3213.

Copy link
Member

Choose a reason for hiding this comment

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

I saw the Cockatrice attribute, but since we don't have it now, I've merged this PR as is. I'll search for all mentions of Cockatrice in C# node anyway while implementing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnnaShaleva AnnaShaleva AnnaShaleva approved these changes

@roman-khimov roman-khimov roman-khimov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose committee address via native Neo contract
3 participants
@AliceInHunterland @AnnaShaleva @roman-khimov