Skip to content

Conversation

@leoslr
Copy link
Contributor

@leoslr leoslr commented Jan 3, 2023

Description of the changes

Small refacto to regroup logic related to eip1559 compliance
Asana task

@coveralls
Copy link

coveralls commented Jan 3, 2023

Coverage Status

Coverage: 88.009% (-0.08%) from 88.088% when pulling b4e6a03 on eip1559-compliance-check into ae5862a on master.

return provider;
};

const isEip1559Supported = async (provider: any, logger: LogTypes.ILogger): Promise<boolean> => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As soon as I try to use the correct type providers.JsonRpcProvider it raises the following TS error:

The inferred type of 'default' cannot be named without a reference to 'ethers/node_modules/@ethersproject/providers'. This is likely not portable. A type annotation is necessary.

The only solution I found to make this go away is to have a separate file dedicated to this function only. Not sure it's worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

a hack you can do is to type it as providers.Provider, then cast it to JsonRpcProvider.

Not great, but still better than any ;)

@leoslr leoslr marked this pull request as ready for review January 3, 2023 18:14
Comment on lines 157 to 159
logger.warn(
'This RPC provider does not support the "eth_feeHistory" method: switching to legacy gas price',
);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this log should be removed from the utility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept it there but made it optional.

return provider;
};

const isEip1559Supported = async (provider: any, logger: LogTypes.ILogger): Promise<boolean> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

a hack you can do is to type it as providers.Provider, then cast it to JsonRpcProvider.

Not great, but still better than any ;)

Copy link
Contributor

@alexandre-abrioux alexandre-abrioux left a comment

Choose a reason for hiding this comment

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

nice!

@leoslr
Copy link
Contributor Author

leoslr commented Jan 4, 2023

@benjlevesque this does not work. As soon as the type of the parameter is related to the ethers library it raise the error, it's related to how we export functions in the utils package.

@benjlevesque
Copy link
Contributor

@benjlevesque this does not work. As soon as the type of the parameter is related to the ethers library it raise the error, it's related to how we export functions in the utils package.

OK, so maybe #1023 will solve it...

@leoslr leoslr merged commit 0dc67c1 into master Jan 18, 2023
@leoslr leoslr deleted the eip1559-compliance-check branch January 18, 2023 13:27
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.

6 participants