Skip to content

Add GasFeeController #494

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

Merged
brad-decker merged 46 commits into from
Jul 7, 2021
Merged

Add GasFeeController #494

Changes from 1 commit
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
a71c93e
First draft of gas fee controller
danjm Jun 14, 2021
c4a2606
Second draft of gas fee controller
danjm Jun 16, 2021
7886180
Third draft of gas fee controller
danjm Jun 17, 2021
2be94cb
Delete use of legacy gas fee
danjm Jun 18, 2021
ba8e275
Track whether polling should be instantiated or stopped with tokens
danjm Jun 18, 2021
179dee6
Network controller getEIP1559Compatibility can be called whenever a n…
danjm Jun 18, 2021
55d3607
update tests (#495)
rickycodes Jun 18, 2021
16075b0
Fetch estimate using eth_gasPrice if on a custom network
danjm Jun 21, 2021
b70d323
getGasFeeEstimatesAndStartPolling returns poll token, and a new one i…
danjm Jun 21, 2021
0a8a248
Add getTimeEstimate to GasFee controller
danjm Jun 21, 2021
cb88478
export GasFeeController
rickycodes Jun 21, 2021
1ccd62a
Add public method for calling and returning gas estimates without pol…
danjm Jun 23, 2021
dbf9f6c
Fix return of fetchLegacyGasPriceEstimate
danjm Jun 23, 2021
16b313b
Proper error handling and fallback for _fetchGasFeeEstimateData
danjm Jun 23, 2021
a782c9d
Include estimated time bounds in gas fee state
danjm Jun 23, 2021
32b2930
rename
rickycodes Jun 23, 2021
97e9e8f
Add GasFeeController.test.ts
rickycodes Jun 23, 2021
47f3659
remove TODO
rickycodes Jun 24, 2021
9971e68
Add result token length test
rickycodes Jun 24, 2021
91a2cd2
Add estimates property test
rickycodes Jun 24, 2021
0dec6e7
Add should fail to re-initialize test
rickycodes Jun 24, 2021
e4e962e
remove console.log
rickycodes Jun 24, 2021
afdc385
do not modify state directly
rickycodes Jun 24, 2021
fd4bd64
Use before/afterEach and fix messenger
rickycodes Jun 24, 2021
b1c5631
check gasFeeEstimates properties
rickycodes Jun 24, 2021
3646d62
check that state is empty to start
rickycodes Jun 24, 2021
51dad0d
Add one additional property check
rickycodes Jun 24, 2021
4ac4ffe
Adding TokenListController to fetch the token list from token service…
NiranjanaBinoy Jun 28, 2021
c1ca513
address feedback
rickycodes Jun 30, 2021
0458679
add mock server for eip1559
rickycodes Jun 15, 2021
728c039
get tests working again
rickycodes Jun 30, 2021
cd90799
Use heroku endpoint
rickycodes Jun 30, 2021
8d570bb
Handle fetch correctly in gasfeecontroller unit tests, using nock
danjm Jun 30, 2021
2e2b1a7
gasFee controller calculateTimeEstimate handles decimals, by way of u…
danjm Jun 30, 2021
1516d87
Lint fix
danjm Jun 30, 2021
6eac11c
Fix dec to gwi (#504)
brad-decker Jul 1, 2021
3df6baf
use ethjs-unit for unit conversions (#506)
brad-decker Jul 1, 2021
117d898
Add metaswaps API and normalize all gas fee units to dec gwei (#507)
brad-decker Jul 1, 2021
b4eebac
update types to be more identifiable (#508)
brad-decker Jul 2, 2021
6fbf024
use LegacyGasFeeEstimate type
brad-decker Jul 2, 2021
5d2908e
handle hex prefixed
brad-decker Jul 5, 2021
283fc88
baseFee -> baseFeePerGas
brad-decker Jul 5, 2021
cf82e70
make more configurable (#513)
brad-decker Jul 6, 2021
8d04cc5
use optional chaining
brad-decker Jul 6, 2021
41cdace
update test case wording
brad-decker Jul 7, 2021
3a42d98
use testdata instead of programmatic testing
brad-decker Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update test case wording
  • Loading branch information
@brad-decker
brad-decker committed Jul 7, 2021
commit 41cdace735728de3790b0475a2a1aff3354b765b
6 changes: 3 additions & 3 deletions src/util.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ describe('util', () => {
});

describe('weiHexToGweiDec', () => {
Copy link
Contributor Author

@danjm danjm Jul 7, 2021

Choose a reason for hiding this comment

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

Might be personal preference, but I think these tests can be clearer in intent, and also less prone to error, if the hard coded inputs and hard coded outputs are explicitly provided, as opposed to be calculated by other functions.

This is how I would write these tests, feel free to copy and paste:

describe('weiHexToGweiDec', () => {
  it('should convert a whole number to WEI', () => {
    const testData = [
      {
        input: '3b9aca00',
        expectedResult: 1,
      },
      {
        input: '1ca35f0e00',
        expectedResult: 123,
      },
      {
        input: '178411b200',
        expectedResult: 101,
      },
      {
        input: '11f5021b400',
        expectedResult: 1234,
      },
    ];
    testData.forEach(({ input, expectedResult }) => {
      expect(
        util.weiHexToGweiDec(input),
      ).toBe(expectedResult);
    });
  });

  it('should convert a number with a decimal part to WEI', () => {
    const testData = [
      {
        input: '4190ab00',
        expectedResult: 1.1,
      },
      {
        input: '1ca3f7a480',
        expectedResult: 123.01,
      },
      {
        input: '178420f440',
        expectedResult: 101.001,
      },
      {
        input: '11f71ed6fc0',
        expectedResult: 1234.567,
      },
    ];

    testData.forEach(({ input, expectedResult }) => {
      expect(
        util.weiHexToGweiDec(input),
      ).toBe(expectedResult);
    });
  });

  it('should convert a number < 1 to WEI', () => {
    const testData = [
      {
        input: '5f5e100',
        expectedResult: 0.1,
      },
      {
        input: '989680',
        expectedResult: 0.01,
      },
      {
        input: 'f4240',
        expectedResult: 0.001,
      },
      {
        input: '21cbbbc0',
        expectedResult: 0.567,
      },
    ];

    testData.forEach(({ input, expectedResult }) => {
      expect(
        util.weiHexToGweiDec(input),
      ).toBe(expectedResult);
    });
  });

  it('should work with 0x prefixed values', () => {
    expect(util.weiHexToGweiDec('0x5f48b0f7')).toBe('1.598599415');
  });
});

Copy link
Member

@wachunei wachunei Jul 7, 2021

Choose a reason for hiding this comment

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

I was not going to comment about this, but jest has a test.each function which is my personal preference, either as string literal table or array table.

Example:

test.each`
  a    | b    | expected
  ${1} | ${1} | ${2}
  ${1} | ${2} | ${3}
  ${2} | ${1} | ${3}
`('returns $expected when $a is added $b', ({a, b, expected}) => {
  expect(a + b).toBe(expected);
});

Example from mobile:
https://github.com/MetaMask/metamask-mobile/blob/978b73d88a5208a7d5d74415af980a2f3a59403d/app/components/Base/Keypad/createKeypadRule.test.js

Copy link
Contributor

@brad-decker brad-decker Jul 7, 2021

Choose a reason for hiding this comment

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

copy and pasted, just had to update the expectedResult to strings

Copy link
Contributor

@brad-decker brad-decker Jul 7, 2021

Choose a reason for hiding this comment

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

3a42d98

it('should convert a whole number to WEI', () => {
it('should convert a WEI hex representing whole amounts of GWEI to whole number', () => {
const numbersInGwei = [1, 123, 101, 1234];
numbersInGwei.forEach((gweiDec) => {
expect(
Expand All @@ -151,7 +151,7 @@ describe('util', () => {
});
});

it('should convert a number with a decimal part to WEI', () => {
it('should convert a WEI hex representing fractional GWEI into a decimal number', () => {
const numbersInGwei = [1.1, 123.01, 101.001, 1234.567];
numbersInGwei.forEach((gweiDec) => {
expect(
Expand All @@ -160,7 +160,7 @@ describe('util', () => {
});
});

it('should convert a number < 1 to WEI', () => {
it('should convert a WEI hex representing values less than 1 GWEI into decimal numbers', () => {
const numbersInGwei = [0.1, 0.01, 0.001, 0.567];
numbersInGwei.forEach((gweiDec) => {
expect(
Expand Down