-
Notifications
You must be signed in to change notification settings - Fork 80
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
chore(deps): upgrade viem to ^2.7.16 #5022
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5022 +/- ##
==========================================
+ Coverage 85.48% 85.52% +0.04%
==========================================
Files 724 724
Lines 29538 29612 +74
Branches 5101 5127 +26
==========================================
+ Hits 25251 25327 +76
- Misses 4048 4050 +2
+ Partials 239 235 -4
... and 27 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
src/viem/estimateFeesPerGas.ts
Outdated
@@ -9,7 +9,7 @@ import { | |||
export async function estimateFeesPerGas( | |||
client: Client, | |||
feeCurrency?: Address | |||
): Promise<{ maxFeePerGas: bigint; maxPriorityFeePerGas: bigint; baseFeePerGas: bigint }> { | |||
): Promise<{ maxFeePerGas: bigint; maxPriorityFeePerGas: bigint; baseFeePerGas: bigint | null }> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
baseFeePerGas
from getBlock
now has a type of BigInt | null
, it used to be any
in prior versions. Looks like this will always be returned for the latest block, it's only null for older blocks before the EIP 1559 upgrade https://www.quicknode.com/docs/ethereum/eth_getBlockByHash. Another option to fix this would be to just throw if this is null
as it would never happen in practice for the latest block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a slight preference for throwing here, since we're only dealing with EIP-1559 based TXs in the rest of the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
src/viem/estimateFeesPerGas.ts
Outdated
@@ -9,7 +9,7 @@ import { | |||
export async function estimateFeesPerGas( | |||
client: Client, | |||
feeCurrency?: Address | |||
): Promise<{ maxFeePerGas: bigint; maxPriorityFeePerGas: bigint; baseFeePerGas: bigint }> { | |||
): Promise<{ maxFeePerGas: bigint; maxPriorityFeePerGas: bigint; baseFeePerGas: bigint | null }> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a slight preference for throwing here, since we're only dealing with EIP-1559 based TXs in the rest of the code.
### Description Includes some typing changes. - WalletActions need a more specific account type, it doesn't like the `undefined` part of `Account | undefined` - `baseFeePerGas` from `getBlock` returns `BigInt | null`, which I believe has always been the case. In previous versions, we were getting back `any` as the type which was assigned to `BigInt`. Now we throw if it's null as it can only be null in blocks prior to the EIP-1559 upgrade ### Test plan CI ### Related issues - Fixes ACT-1111 ### Backwards compatibility Yes
Description
Includes some typing changes.
undefined
part ofAccount | undefined
baseFeePerGas
fromgetBlock
returnsBigInt | null
, which I believe has always been the case. In previous versions, we were getting backany
as the type which was assigned toBigInt
. Now we throw if it's null as it can only be null in blocks prior to the EIP-1559 upgradeTest plan
CI
Related issues
Backwards compatibility
Yes