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

Fix caller write privileges for native cpi #18616

Closed

Conversation

jstarry
Copy link
Member

@jstarry jstarry commented Jul 12, 2021

Problem

Caller write privileges are not created correctly for native cpi. The caller_write_privileges vector should match up with message.account_keys because they will be indexed the same.

Summary of Changes

  • Remove incorrect write privilege value insertion
  • Match up write privileges vector with the accounts vector built from message.account_keys

Fixes #

@jstarry
Copy link
Member Author

jstarry commented Jul 12, 2021

Since native_invoke is only called for the bpf upgradeable program and that program doesn't modify caller accounts before invoking the native create account instruction, there shouldn't be any change of behavior here.

@jackcmay
Copy link
Contributor

Do you have a test that highlights the issue?

@jstarry
Copy link
Member Author

jstarry commented Jul 12, 2021

Not yet, just running through CI first to see what pops up. Should have opened as draft

@codecov
Copy link

codecov bot commented Jul 12, 2021

Codecov Report

Merging #18616 (8cefd9a) into master (3e16df8) will increase coverage by 0.1%.
The diff coverage is 89.8%.

@@            Coverage Diff            @@
##           master   #18616     +/-   ##
=========================================
+ Coverage    82.6%    82.7%   +0.1%     
=========================================
  Files         437      440      +3     
  Lines      122557   123897   +1340     
=========================================
+ Hits       101288   102527   +1239     
- Misses      21269    21370    +101     

@jstarry
Copy link
Member Author

jstarry commented Jul 13, 2021

Hmm looks like there aren't any tests for native cpi yet beyond bpf loader integration tests, I don't have the time to put a test suite together right now so I'll write up an issue instead.

@jstarry jstarry closed this Jul 13, 2021
@jstarry
Copy link
Member Author

jstarry commented Jul 13, 2021

Issue here: #18629

@jstarry jstarry deleted the fix-caller-write-privileges branch November 29, 2021 01:20
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.

2 participants