-
Notifications
You must be signed in to change notification settings - Fork 37
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
Refactor TransactionWatcher to accept either Transaction or hash #404
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.
👍 This undoes a breaking change.
|
||
await Promise.all([ | ||
provider.mockTransactionTimelineByHash(hash, [new Wait(40), new TransactionStatus("pending"), new Wait(40), new TransactionStatus("executed"), new MarkCompleted()]), | ||
watcher.awaitCompleted(dummyTransaction) |
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.
👍
import { ITransactionFetcher } from "./interface"; | ||
import { ITransactionEvent, ITransactionOnNetwork, ITransactionStatus } from "./interfaceOfNetwork"; | ||
import { Logger } from "./logger"; | ||
|
||
export type PredicateIsAwaitedStatus = (status: ITransactionStatus) => boolean; | ||
|
||
/** | ||
* Internal interface: a transaction, as seen from the perspective of a {@link TransactionWatcher}. |
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.
Indeed, for backwards compatibility (legacy).
src/transactionWatcher.ts
Outdated
}; | ||
|
||
const doFetch = async () => await this.fetcher.getTransaction(txHash); | ||
const doFetch = async () => { | ||
const hash = typeof transactionOrTxHash === "string" ? transactionOrTxHash : transactionOrTxHash.getHash().hex(); |
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.
Since logic is in several places, could have been extracted to a private function e.g. transactionOrTxHashToTxHash(...): string
.
Can also stay as it is, though.
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.
extracted to a private method.
src/transactionWatcher.ts
Outdated
} | ||
|
||
private transactionOrTxHashToTxHash(transactionOrTxHash: ITransaction | string): string { | ||
return typeof transactionOrTxHash === "string" ? transactionOrTxHash : transactionOrTxHash.getHash().hex(); |
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.
could also check the validity of the hash. it can be "invalid tx hash" instead of 64 hex chars
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.
Done
The methods of the
TransactionWatcher
can be called using either the transaction object or the hash of the transaction.