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

[EVM] Fix COA ownership proof #6550

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Oct 9, 2024

COA ownership proof verification did not work if the keys used had a non-0 index due to confusing the account key index and the index of the key in the key list.

See discussion: https://discord.com/channels/613813861610684416/1167476806333513800/1293534831036071947

I fixed the issue and (hopefully) made the code a bit more clear.

This is a contract change only, so no software update is needed to deploy this on existing networks, we only need to update the contract there.

@janezpodhostnik janezpodhostnik added the Bug Something isn't working label Oct 9, 2024
@janezpodhostnik janezpodhostnik requested review from a team October 9, 2024 13:44
@janezpodhostnik janezpodhostnik self-assigned this Oct 9, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.09%. Comparing base (efd21c1) to head (abaf989).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6550      +/-   ##
==========================================
- Coverage   41.26%   41.09%   -0.18%     
==========================================
  Files        2031     1993      -38     
  Lines      145883   142819    -3064     
==========================================
- Hits        60204    58696    -1508     
+ Misses      79459    78076    -1383     
+ Partials     6220     6047     -173     
Flag Coverage Δ
unittests 41.09% <ø> (-0.18%) ⬇️

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.

@ramtinms
Copy link
Member

Do we want to create a new PR for porting the changes deployed and maybe we close this one ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants