Fix TransactionResult type - #125
Conversation
7c7b534 to
03208a9
Compare
Alan Lu (cag)
left a comment
Alan Lu (cag)
left a comment
There was a problem hiding this comment.
Merging as is since detecting contract versions is built on the tip of this branch, and there's no glaring issues, but made the comments for reference later.
|
|
||
| toSafeRelayTxResult(txHash: string, tx: Record<string, any>): Promise<Web3TransactionResult> { | ||
| tx['transactionHash'] = tx['txHash'] | ||
| delete tx['txHash'] |
There was a problem hiding this comment.
Note to self: this is done to make the result of the relay look like a web3 transaction result.
|
|
||
| export interface Web3TransactionResult extends SimpleTransactionResult { | ||
| sendOptions?: SendOptions | ||
| promiEvent: Promise<any> |
There was a problem hiding this comment.
Not sure if promiEvent should be typed as a Promise, as I've had issues directly awaiting it in the past.
| sendOptions?: SendOptions | ||
| promiEvent?: any | ||
| transactionResponse?: Record<string, any> | ||
| } |
There was a problem hiding this comment.
I was under the impression that TransactionResult would be the only transaction result type from this, but instead it looks like there are now 4 different transaction result types? Couldn't those other types be removed with this being the only type?
Alternatively, with this approach, could use multiple inheritance:
export interface TransactionResult extends Web3TransactionResult, EthersTransactionResult {}| } | ||
| } | ||
| }, | ||
| waitTxReceipt: async (txResult: TransactionResult): Promise<any> => { |
There was a problem hiding this comment.
Should check if waitTxReceipt is still used.
| } | ||
| return receipt | ||
| }, | ||
| waitSafeTxReceipt: async (txResult: TransactionResult): Promise<any> => { |
There was a problem hiding this comment.
Maybe should be a Promise<Record<string, any> | undefined>, or even a Promise<Record<string, any>> if returning undefined can get changed to throwing an error.
2 participants
Check #118