-
Notifications
You must be signed in to change notification settings - Fork 323
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
fix: custom fallback transport #12470
base: master
Are you sure you want to change the base?
Conversation
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.
Looks good! Just threw in a few comments. Also sent a quick PR to viem to see if they can add shouldThrow
as a config.
@@ -55,7 +55,7 @@ import { Attributes, type TelemetryClient, type Traceable, type Tracer, trackSpa | |||
|
|||
import { EventEmitter } from 'events'; | |||
import groupBy from 'lodash.groupby'; | |||
import { type GetContractReturnType, createPublicClient, fallback, getContract, http } from 'viem'; | |||
import { type GetContractReturnType, createPublicClient, getContract, http } from 'viem'; |
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.
Let's add a rule to eslint so we don't accidentally import viem's fallback
instead of our own: https://eslint.org/docs/latest/rules/no-restricted-imports#importnames
import type { ExtendedViemWalletClient } from '../types.js'; | ||
import { startAnvil } from './start_anvil.js'; | ||
|
||
describe('fallback_transport', () => { |
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.
Let's add a test for the transport not falling back on a contract error (main reason why you implemented this as far as I understand!)
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.
ofc 🤦
fallback
transport by viem works by trying the first node it was given, and if it receives an error, it moves on to the next, unlessshouldThrow
returnstrue
.shouldThrow
however was not triggered on contract errors so with checks likecanProposeAt
, it would try secondary nodes, even though the 1st node returned the correct result (a contract error)This implementation of
fallback
transport throws immediately when we receive any contract error