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

Ensure tx has value before it's added #8486

Merged
merged 1 commit into from
May 1, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Apr 30, 2020

Previously a transaction would get assigned a default value during the addTxGasDefaults function, after the transaction was added and sent to the UI.

Instead the transaction is assigned a default value before it gets added. This flow is simpler to follow, and it avoids the race condition where the transaction is assigned a value from the UI before this default is set. In that situation, the UI-assigned value would be overridden, which is obviously not desired.

@Gudahtt Gudahtt force-pushed the ensure-tx-has-value-before-adding branch from 7f85a81 to 8c472e6 Compare April 30, 2020 23:59
Previously a transaction would get assigned a default value during the
`addTxGasDefaults` function, after the transaction was added and sent
to the UI.

Instead the transaction is assigned a default value before it gets
added. This flow is simpler to follow, and it avoids the race condition
where the transaction is assigned a value from the UI before this
default is set. In that situation, the UI-assigned value would be
overridden, which is obviously not desired.
@Gudahtt Gudahtt force-pushed the ensure-tx-has-value-before-adding branch from 8c472e6 to 61f7b31 Compare May 1, 2020 00:02
@Gudahtt Gudahtt marked this pull request as ready for review May 1, 2020 00:25
@Gudahtt Gudahtt requested a review from a team as a code owner May 1, 2020 00:25
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [61f7b31]
Page Load Metrics (639 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint2910339157
domContentLoaded6026746372311
load6046766392311
domInteractive6026746372311

@Gudahtt Gudahtt merged commit 92592fc into develop May 1, 2020
@Gudahtt Gudahtt deleted the ensure-tx-has-value-before-adding branch May 1, 2020 00:50
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.

3 participants