Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

@0x/contracts-extensions: Maximum gas price contract #2511

Merged
merged 5 commits into from
Mar 5, 2020

Conversation

moodlezoup
Copy link
Contributor

@moodlezoup moodlezoup commented Mar 4, 2020

Description

Contract includes a parameter-less version which uses a hard-coded max of 20 Gwei, and a version that allows makers to specify a custom max value.
Includes tooling for encoding/decoding the StaticCall asset data, as well as some basic unit tests

Testing instructions

Types of changes

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@moodlezoup moodlezoup force-pushed the feature/extensions/max-gasprice-orders branch from ebf640a to 9c5ac26 Compare March 4, 2020 01:31
@moodlezoup moodlezoup changed the title Feature/extensions/max gasprice orders @0x/contracts-extensions: Maximum gas price contract Mar 4, 2020
@moodlezoup moodlezoup marked this pull request as ready for review March 4, 2020 01:34

*/

pragma solidity ^0.5.16;
Copy link
Contributor

Choose a reason for hiding this comment

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

👀

@coveralls
Copy link

coveralls commented Mar 4, 2020

Coverage Status

Coverage decreased (-0.1%) to 79.588% when pulling 6f5bf9d on feature/extensions/max-gasprice-orders into 7b8c834 on development.

/// @dev Checks that the current transaction's gas price is less than
/// the specified maximum value.
/// @param data Encodes the maximum gas price.
function checkGasPrice(bytes calldata data)
Copy link
Member

Choose a reason for hiding this comment

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

Why not just take a uint256 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't realize that would work 🙈

*/

pragma solidity ^0.5.16;
pragma experimental ABIEncoderV2;
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we actually need to use ABIv2 here. I'd just run tests with/without to see if there are any gas differences.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like without is slightly cheaper for the parametrized version

* Encodes the given stop limit data parameters into StaticCall asset data so that it can be used
* in a 0x order.
*/
export function encodeMaxGasPriceStaticCallData(maxGasPriceContractAddress: string, maxGasPrice?: BigNumber): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for writing this helper 🙌

external
view
Copy link
Member

Choose a reason for hiding this comment

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

Why did we get rid of view here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops that was a remnant of some tests

@moodlezoup moodlezoup force-pushed the feature/extensions/max-gasprice-orders branch from 4af2577 to 4900949 Compare March 5, 2020 06:12
@moodlezoup moodlezoup merged commit bcd9247 into development Mar 5, 2020
@moodlezoup moodlezoup deleted the feature/extensions/max-gasprice-orders branch March 5, 2020 20:30
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.

5 participants