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

wallet_contract now returns contract based on the hash #12041

Merged
merged 4 commits into from
Sep 4, 2024

Conversation

nagisa
Copy link
Collaborator

@nagisa nagisa commented Sep 4, 2024

Hash is stored in the Account and the rest of the codebase largely expects the hash to uniquely identify the specific contract code. For the time being just return exactly what's stored in the Account and think about updating the accounts later...

Hash is stored in the `Account` and the rest of the codebase largely
expects the hash to uniquely identify the specific contract code. For
the time being just return exactly what's stored in the `Account` and
think about updating the accounts later...
@nagisa nagisa requested a review from a team as a code owner September 4, 2024 11:40
Copy link

codecov bot commented Sep 4, 2024

Codecov Report

Attention: Patch coverage is 65.38462% with 9 lines in your changes missing coverage. Please review.

Project coverage is 71.50%. Comparing base (0e6179a) to head (b32da55).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
runtime/near-wallet-contract/src/lib.rs 57.89% 5 Missing and 3 partials ⚠️
runtime/runtime/src/ext.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12041      +/-   ##
==========================================
- Coverage   71.51%   71.50%   -0.02%     
==========================================
  Files         814      814              
  Lines      164513   164465      -48     
  Branches   164513   164465      -48     
==========================================
- Hits       117648   117593      -55     
- Misses      41699    41703       +4     
- Partials     5166     5169       +3     
Flag Coverage Δ
backward-compatibility 0.17% <0.00%> (-0.01%) ⬇️
db-migration 0.17% <0.00%> (-0.01%) ⬇️
genesis-check 1.28% <0.00%> (-0.01%) ⬇️
integration-tests 38.59% <65.38%> (+0.01%) ⬆️
linux 71.29% <65.38%> (-0.02%) ⬇️
linux-nightly 71.07% <65.38%> (-0.03%) ⬇️
macos 54.17% <0.00%> (+0.07%) ⬆️
pytests 1.54% <0.00%> (-0.01%) ⬇️
sanity-checks 1.34% <0.00%> (-0.01%) ⬇️
unittests 65.36% <3.84%> (-0.04%) ⬇️
upgradability 0.21% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link
Contributor

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

The general idea looks right to me. It would be nice to restore the tests in the wallet contract crate in a follow-up PR.

Copy link
Contributor

@birchmd birchmd left a comment

Choose a reason for hiding this comment

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

Sorry, I realized this change isn't quite right because of the legacy accounts that look like eth-implicit, but are not.

runtime/runtime/src/ext.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jancionear jancionear left a comment

Choose a reason for hiding this comment

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

lgtm for testnet, cherry-picking...

@nagisa nagisa added this pull request to the merge queue Sep 4, 2024
Merged via the queue into near:master with commit e8fa073 Sep 4, 2024
27 of 29 checks passed
@nagisa nagisa deleted the fixes-wallet-contract branch September 4, 2024 15:32
}
_ => LOCALNET.read_contract(),
Copy link
Contributor

@VanBarbascu VanBarbascu Sep 4, 2024

Choose a reason for hiding this comment

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

This allowed the LOCALNET contract to be deployed on test chains. i.e. mocknet. What contract will be picked in this case?

staffik pushed a commit that referenced this pull request Sep 5, 2024
Hash is stored in the `Account` and the rest of the codebase largely
expects the hash to uniquely identify the specific contract code. For
the time being just return exactly what's stored in the `Account` and
think about updating the accounts later...
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.

5 participants