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

Conversation

@jackcmay
Copy link
Contributor

@jackcmay jackcmay commented Jul 6, 2021

Problem

Update the owner and the executable at the same time

Summary of Changes

Catch in the verify stage

Fixes #

@codecov
Copy link

codecov bot commented Jul 7, 2021

Codecov Report

Merging #18459 (dc640b6) into master (d451363) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #18459     +/-   ##
=========================================
- Coverage    82.5%    82.5%   -0.1%     
=========================================
  Files         436      436             
  Lines      121850   121851      +1     
=========================================
- Hits       100605   100597      -8     
- Misses      21245    21254      +9     

@jackcmay jackcmay merged commit 44289e6 into solana-labs:master Jul 7, 2021
mergify bot pushed a commit that referenced this pull request Jul 7, 2021
(cherry picked from commit 44289e6)

# Conflicts:
#	runtime/src/message_processor.rs
mergify bot pushed a commit that referenced this pull request Jul 7, 2021
(cherry picked from commit 44289e6)
mergify bot added a commit that referenced this pull request Jul 7, 2021
* Update verify policy (#18459)

(cherry picked from commit 44289e6)

# Conflicts:
#	runtime/src/message_processor.rs

* resolve conflicts

Co-authored-by: Jack May <[email protected]>
mergify bot added a commit that referenced this pull request Jul 7, 2021
* Update verify policy (#18459)

(cherry picked from commit 44289e6)

* resolve conflicts

Co-authored-by: Jack May <[email protected]>
if !is_writable // line coverage used to get branch coverage
|| pre.executable()
|| program_id != pre.owner()
|| program_id != post.owner()
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need gated?

Copy link
Contributor

Choose a reason for hiding this comment

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

n/m. I see #18492 now 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, totally missed that in review. Needs gating because you could set executable to be true and get a successful transaction right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, realized that as soon as I hit merge ;-).

This covers it: #18492

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, totally missed that in review. Needs gating because you could set executable to be true and get a successful transaction right?

These get easier to see the more cluster deaths you cause 🙂

@jackcmay jackcmay deleted the update-verify-policy branch July 7, 2021 20:18
This was referenced Aug 23, 2021
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.

3 participants