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.

Sign up for GitHub

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

Jump to bottom

simulateTransaction overriding recentBlockhash is destructive #24279

Closed
dvcrn opened this issue Apr 12, 2022 · 1 comment
Closed

simulateTransaction overriding recentBlockhash is destructive #24279

dvcrn opened this issue Apr 12, 2022 · 1 comment

Comments

@dvcrn
Copy link
Contributor

dvcrn commented Apr 12, 2022 •
edited

Problem

After hours of debugging weird issues with transactions changing in between adding signatures we finally figured out that simulateTransaction is overriding recentBlockhash whenever it's used, which caused our Transaction object to get a new recentBlockhash because the same reference was used:

https://github.com/solana-labs/solana/blob/master/web3.js/src/connection.ts#L3891=

blockhash is needed to simulate properly, but overriding of an existing blockhash was not expected behavior

Proposed Solution

Simulating transactions should be non-destructive. Some ideas how to solve this:

  • Throw error when a recentBlockhash is not set (eg. do nothing)
  • Set only if not set yet
  • Set only when populating from Message (creating a new instance altogether)
  • Make a copy of the Transaction instance
dvcrn added a commit to dvcrn/solana that referenced this issue Apr 12, 2022
@dvcrn
Update simulate to add blockhash if not exist
996508f 
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
@dvcrn dvcrn mentioned this issue Apr 12, 2022
Merged
jstarry pushed a commit that referenced this issue Apr 13, 2022
@dvcrn
fix: don't override a transaction's recentBlockhash when calling simu…
d8c45a6 
…late if it's already set (#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 #24279

* Apply prettier
mvines pushed a commit to solana-labs/solana-web3.js that referenced this issue Apr 13, 2022
@dvcrn
fix: don't override a transaction's recentBlockhash when calling simu…
e707c2b 
…late if it's already set (#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/solana#24279

* Apply prettier
@jstarry
Copy link
Member

jstarry commented Apr 13, 2022

Closed by #24280

@jstarry jstarry closed this as completed Apr 13, 2022
jeffwashington pushed a commit to jeffwashington/solana that referenced this issue Jun 29, 2022
@dvcrn @jeffwashington
fix: don't override a transaction's recentBlockhash when calling simu…
73fed0b 
…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 issue Jun 30, 2022
@dvcrn @jeffwashington
fix: don't override a transaction's recentBlockhash when calling simu…
d25cf7c 
…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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dvcrn @jstarry