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

Allow for speedUp and stop based on provided gasValues from consumer #535

Merged
merged 23 commits into from
Aug 11, 2021

Conversation

rickycodes
Copy link
Contributor

@rickycodes rickycodes commented Jul 22, 2021

opening this as a draft as it relies on #521

this adds a new interface GasValues which can be used to dynamically set values for speedUp and stopTransaction which can be used instead of our previously existing fixed rates.

this will allow consumers to set custom gas values (in hex) for both those actions.

Base automatically changed from eip1559-speed-up to main July 23, 2021 15:37
@rickycodes rickycodes marked this pull request as ready for review July 23, 2021 16:19
@rickycodes rickycodes requested a review from a team as a code owner July 23, 2021 16:19
src/util.ts Outdated
const keys = Object.keys(gasValues);
keys.forEach((key) => {
const val = (gasValues as any)[key];
if (typeof val !== 'string' || !isHexString(val)) {
Copy link
Contributor Author

@rickycodes rickycodes Jul 27, 2021

Choose a reason for hiding this comment

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

technically this typeof check is redundant since you can't set these values to anything other than string. That said, I don't think it hurts to have an additional check.

src/util.ts Outdated Show resolved Hide resolved
src/transaction/TransactionController.ts Outdated Show resolved Hide resolved
…to dynamic-speed-up

* 'dynamic-speed-up' of github.com:MetaMask/controllers:
  Bump @metamask/auto-changelog from 2.4.0 to 2.5.0 (#549)
  Update Changelog (#548)
  14.0.2 (#547)
  Fix `resetPolling` functionality (#546)
  TokenService improvements (#541)
  Release/14.0.1 (#545)
  Make gweiDecToWEIBN util resilient against params with too many decimals (#544)
  Bump @metamask/contract-metadata from 1.27.0 to 1.28.0 (#540)
  14.0.0 (#539)
  Bug: Mainnet NFT Autodetect API  (#536)
ensure tx is increased by at least the minimum required amount
@rickycodes rickycodes requested a review from Gudahtt August 4, 2021 18:47
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

Looks good so far! Just the missing validation in stopTransaction

src/transaction/TransactionController.ts Show resolved Hide resolved
@rickycodes
Copy link
Contributor Author

Thanks @Gudahtt! I've added the validation in stopTransaction now as well.

…to dynamic-speed-up

* 'dynamic-speed-up' of github.com:MetaMask/controllers:
  Consolidate token list controller data (#527)
  14.1.0 (#553)
  Feature: Add selector subscriptions (#551)
@rickycodes
Copy link
Contributor Author

@Gudahtt fixed spelling and removed unnecessary optional chaining

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@rickycodes rickycodes merged commit e35d438 into main Aug 11, 2021
@rickycodes rickycodes deleted the dynamic-speed-up branch August 11, 2021 15:18
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
…535)

* Allow for speedUp and stop based on provided gasValues from consumer

* make this optional

* validate gas values are hex string

* use util

* rename

* update wording

* Update tests

* remove return true

* Use type predicate functions

* Move type predicate functions to util

* Update unit tests

* Add validation
ensure tx is increased by at least the minimum required amount

* Add validateMinimumIncrease method

* Address nit

* Add validation in stopTransaction

* fix spelling

* remove unnecessary optional chaining

* test
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
…535)

* Allow for speedUp and stop based on provided gasValues from consumer

* make this optional

* validate gas values are hex string

* use util

* rename

* update wording

* Update tests

* remove return true

* Use type predicate functions

* Move type predicate functions to util

* Update unit tests

* Add validation
ensure tx is increased by at least the minimum required amount

* Add validateMinimumIncrease method

* Address nit

* Add validation in stopTransaction

* fix spelling

* remove unnecessary optional chaining

* test
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.

2 participants