Skip to content

cli: allow dynamic contract hash for contract bindings #3405

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

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

AliceInHunterland
Copy link
Contributor

@AliceInHunterland AliceInHunterland commented Apr 8, 2024

Close #3007

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.

Adjust commit and PR title, it's not quite correct.

@AliceInHunterland AliceInHunterland changed the title smartcontract: add dynamic hash cli: add dynamic binding contract hash Apr 8, 2024
@AliceInHunterland AliceInHunterland force-pushed the dynamic-hash branch 2 times, most recently from 0e92b47 to 63f7877 Compare April 8, 2024 22:14
@AliceInHunterland AliceInHunterland force-pushed the dynamic-hash branch 2 times, most recently from 3874358 to e0aec55 Compare April 8, 2024 22:33
@AliceInHunterland AliceInHunterland marked this pull request as ready for review April 8, 2024 22:34
@AnnaShaleva AnnaShaleva changed the title cli: add dynamic binding contract hash cli: allow dynamic contract hash for contract bindings Apr 9, 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.

s/cli: add dynamic binding contract hash/smartcontract: support dynamic contract hash for bindings

Or something like that.

@AnnaShaleva
Copy link
Member

I would like to ensure that generated wrapper with dynamic hash is valid. Can we create a test (using neotest): generate wrapper with dynamic hash for some test contract (we may reuse one of the already existing test contracts), create a tiny helper contract that uses this wrapper inside some simple API (that creates an instance of Contract and calls its method):

func CallWrapper(h interop.Hash160) int {
    ...
}

Then deploy both contract and helper, then call helper and check that everything works as expected.

@AliceInHunterland AliceInHunterland force-pushed the dynamic-hash branch 2 times, most recently from 5bb8429 to 05d5265 Compare April 10, 2024 10:26
@AliceInHunterland AliceInHunterland force-pushed the dynamic-hash branch 2 times, most recently from fafb89a to 40426e6 Compare April 10, 2024 13:26
Copy link

codecov bot commented Apr 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.09%. Comparing base (8913542) to head (c5dbecb).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3405      +/-   ##
==========================================
+ Coverage   86.08%   86.09%   +0.01%     
==========================================
  Files         331      331              
  Lines       38107    38106       -1     
==========================================
+ Hits        32805    32808       +3     
+ Misses       3785     3782       -3     
+ Partials     1517     1516       -1     

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

@AliceInHunterland AliceInHunterland force-pushed the dynamic-hash branch 2 times, most recently from d8a3497 to 7c63898 Compare April 10, 2024 13:55
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.

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.

Change the commit message: Add an option to contract wrappers with dynamic hashes. It's not quite correct, because there were no contract wrappers with dynamic hashes before this PR.

Otherwise LGTM.

Allow dynamic contract hash for contract bindings.

Close #3007

Signed-off-by: Ekaterina Pavlova <ekt@morphbits.io>
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.

@roman-khimov, ready for review if you'd like to.

@roman-khimov roman-khimov merged commit 1786136 into master Apr 15, 2024
20 checks passed
@roman-khimov roman-khimov deleted the dynamic-hash branch April 15, 2024 10:16
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.

Contract wrappers with dynamic hashes
3 participants