Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

NEO candidate registration via NEP-27 callback #3700

Merged
merged 3 commits into from
Dec 25, 2024

Conversation

roman-khimov
Copy link
Member

Copy link

codecov bot commented Nov 24, 2024

Codecov Report

Attention: Patch coverage is 82.05128% with 7 lines in your changes missing coverage. Please review.

Project coverage is 83.06%. Comparing base (dc747ce) to head (38b9b13).
Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
pkg/core/native/native_neo.go 82.05% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3700      +/-   ##
==========================================
- Coverage   83.10%   83.06%   -0.04%     
==========================================
  Files         335      335              
  Lines       46821    46855      +34     
==========================================
+ Hits        38911    38922      +11     
- Misses       6317     6343      +26     
+ Partials     1593     1590       -3     

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

@roman-khimov roman-khimov added the blocked Can't be done because of something label Nov 24, 2024
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.

Functionality itself LGTM, but don't we want to upgrade neo-go wallet candidate vote command as far? We may add a switch to use transfer instead of a call to "vote" if Echidna is enabled on server. Although it may be a matter of a separate issue, create one?

@AnnaShaleva
Copy link
Member

don't we want to upgrade neo-go wallet candidate vote command as far?

Ref. #3775, this PR is only about contract-related changes.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov force-pushed the nep-27-candidate-registration branch from e80e08d to f0457ab Compare December 24, 2024 15:03
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.

We may merge it right now, or may wait for neo-project/neo#3643, @roman-khimov, it's up to you.

Solves two problems:
 * inability to estimate GAS needed for registerCandidate in a regular way
   because of its very high fee (more than what normal RPC servers allow)
 * inability to have MaxBlockSystemFee lower than the registration price
   which is very high on its own (more than practically possible to execute)

See neo-project/neo#3552.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
Connection succeeds when AIO is running, some more obscure port is less likely
to cause this problem.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
@roman-khimov roman-khimov force-pushed the nep-27-candidate-registration branch from f0457ab to 38b9b13 Compare December 25, 2024 07:36
@roman-khimov
Copy link
Member Author

neo-project/neo#3643 will require some additional support from manifest generator and it's not yet there. Let's sync with the current state of things and standard can be added later.

@AnnaShaleva AnnaShaleva merged commit 64c4de4 into master Dec 25, 2024
32 of 34 checks passed
@AnnaShaleva AnnaShaleva deleted the nep-27-candidate-registration branch December 25, 2024 07:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants