Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Track account writable deescalation#14626

Merged
jackcmay merged 1 commit intosolana-labs:masterfrom
jackcmay:track-write-account-deescalation
Jan 22, 2021
Merged

Track account writable deescalation#14626
jackcmay merged 1 commit intosolana-labs:masterfrom
jackcmay:track-write-account-deescalation

Conversation

@jackcmay
Copy link
Copy Markdown
Contributor

Problem

If a program passes a readwrite account as readonly to an inner instruction, then any writes should cause an error. Since the runtime doesn't track changes to write deescalation, it allows those instructions to succeed but will not return those state changes to the caller program

Summary of Changes

Track the writable flag through the call flow

Fixes #13908

@jackcmay jackcmay force-pushed the track-write-account-deescalation branch from 0dc645f to 8a14638 Compare January 18, 2021 17:32
@jackcmay
Copy link
Copy Markdown
Contributor Author

jackcmay commented Jan 19, 2021

@mvines Looks like SPL Feature Proposal breaks with these changes. Is it trying to write to accounts that have been passed as read-only via cpi?

@jackcmay jackcmay added the v1.5 label Jan 22, 2021
@jackcmay jackcmay force-pushed the track-write-account-deescalation branch from 8a14638 to 8ce0edd Compare January 22, 2021 21:46
@jackcmay jackcmay force-pushed the track-write-account-deescalation branch from 8ce0edd to 6488428 Compare January 22, 2021 21:48
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 22, 2021

Codecov Report

Merging #14626 (6488428) into master (cbb9ac1) will increase coverage by 0.0%.
The diff coverage is 69.8%.

@@           Coverage Diff           @@
##           master   #14626   +/-   ##
=======================================
  Coverage    80.3%    80.3%           
=======================================
  Files         403      403           
  Lines      102296   102343   +47     
=======================================
+ Hits        82196    82238   +42     
- Misses      20100    20105    +5     

@jackcmay jackcmay merged commit 77572a7 into solana-labs:master Jan 22, 2021
@jackcmay jackcmay deleted the track-write-account-deescalation branch January 22, 2021 23:28
mergify Bot pushed a commit that referenced this pull request Jan 22, 2021
(cherry picked from commit 77572a7)

# Conflicts:
#	sdk/src/feature_set.rs
mergify Bot added a commit that referenced this pull request Jan 23, 2021
* Track account writable deescalation (#14626)

(cherry picked from commit 77572a7)

# Conflicts:
#	sdk/src/feature_set.rs

* fix conflicts

Co-authored-by: Jack May <jack@solana.com>
.iter()
.map(|keyed_account| keyed_account.is_writable())
.collect::<Vec<bool>>();
caller_privileges.insert(0, false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jackcmay why insert false here? And isn't caller_privileges supposed to be aligned with accounts rather than keyed_accounts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

IIRC it holds the space of the program, which isn't part of the keyed_accounts and the program is not writeable

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Got it, it doesn't look like that's the case anymore on latest master so I have a fix here: #18616

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Account readwrite deescalation is not persisted in instruction execution context

2 participants