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: don't override a transaction's recentBlockhash when calling simulate if it's already set #24280

Merged
merged 2 commits into from
Apr 13, 2022

Conversation

dvcrn
Copy link
Contributor

@dvcrn dvcrn commented Apr 12, 2022

Simulate has been overriding the recentBlockhash of the passed
Transaction which can be considered destructive and with side effects.

Since the purpose of this function is to purely simulate, it should not
override recentBlockhash if it has already been set

Refs #24279

Simulate has been overriding the recentBlockhash of the passed
Transaction which can be considered destructive and with side effects.

Since the purpose of this function is to purely simulate, it should not
override recentBlockhash if it has already been set

Refs solana-labs#24279
@mergify mergify bot added the community Community contribution label Apr 12, 2022
@jstarry jstarry changed the title Update simulate to add recentBlockhash only if it's not set yet fix: don't override a transaction's recentBlockhash when calling simulate if it's already set Apr 12, 2022
@jstarry
Copy link
Member

jstarry commented Apr 12, 2022

Can you run prettier? And don't worry about the commit lint

@dvcrn
Copy link
Contributor Author

dvcrn commented Apr 13, 2022

Prettier'd!

@codecov
Copy link

codecov bot commented Apr 13, 2022

Codecov Report

Merging #24280 (8e210af) into master (2da4e3e) will decrease coverage by 11.6%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master   #24280       +/-   ##
===========================================
- Coverage    81.8%    70.1%    -11.7%     
===========================================
  Files         581       36      -545     
  Lines      158312     2236   -156076     
  Branches        0      316      +316     
===========================================
- Hits       129518     1568   -127950     
+ Misses      28794      558    -28236     
- Partials        0      110      +110     

@jstarry jstarry merged commit d8c45a6 into solana-labs:master Apr 13, 2022
@jstarry
Copy link
Member

jstarry commented Apr 13, 2022

Thanks! A patch release will be rolled out shortly

@mergify mergify bot requested a review from a team April 13, 2022 04:37
@jstarry jstarry requested review from jstarry and removed request for a team April 13, 2022 04:38
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 29, 2022
…late if it's already set (solana-labs#24280)

* Update simulate to add blockhash if not exist

Simulate has been overriding the recentBlockhash of the passed
Transaction which can be considered destructive and with side effects.

Since the purpose of this function is to purely simulate, it should not
override recentBlockhash if it has already been set

Refs solana-labs#24279

* Apply prettier
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Jun 30, 2022
…late if it's already set (solana-labs#24280)

* Update simulate to add blockhash if not exist

Simulate has been overriding the recentBlockhash of the passed
Transaction which can be considered destructive and with side effects.

Since the purpose of this function is to purely simulate, it should not
override recentBlockhash if it has already been set

Refs solana-labs#24279

* Apply prettier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants