test: fix with custom transport #755
Conversation
test/relayFeeCalculator.test.ts
Outdated
| @@ -74,6 +75,7 @@ class ExampleQueries implements QueryInterface { | |||
| constructor(private defaultGas = "305572") {} | |||
|
|
|||
| getGasCosts(): Promise<TransactionCostEstimate> { | |||
| console.log("#####"); | |||
There was a problem hiding this comment.
fwiw you can use the logger in test by running LOG_IN_TEST=yes yarn test ....
| options: Partial<{ | ||
| gasPrice: BigNumberish; | ||
| gasUnits: BigNumberish; | ||
| omitMarkup: boolean; | ||
| transport: Transport; | ||
| }> = {} |
There was a problem hiding this comment.
+1 for this approach - I really think it improves usability to eliminate the need for long convoluted sets of parameters.
|
|
||
| // This needs to be implemented for every chain and passed into RelayFeeCalculator | ||
| export interface QueryInterface { | ||
| getGasCosts: ( | ||
| deposit: Deposit, | ||
| relayer: string, | ||
| gasPrice?: BigNumberish, | ||
| gasLimit?: BigNumberish | ||
| options?: Partial<{ gasPrice: BigNumberish; gasUnits: BigNumberish; omitMarkup: boolean; transport: Transport }> |
There was a problem hiding this comment.
I think it'd be nice to avoid having Transport bleed out into the QueryInterface definitions. I'm not sure if that's acheivable atm and it may not be actionable right now. But in general I think it would be good to keep the implementation choices of the gas price oracle defined internally - especially since it's only really used for test (...it might hint at the need to define a special class for test? idk).
There was a problem hiding this comment.
So currently we use getGasCosts directly in the API. That's why I had to expose it here 🤔
There was a problem hiding this comment.
But yea agree it would be nicer to keep it internal
No description provided.