Adds a transaction timer class to reuse #4852
Conversation
| ) { | ||
| this.logger = logger ?? createRootLogger() | ||
| this.estimateInMs = Math.max(Math.ceil(spendPostTime * raw.spends.length), 1000) | ||
| options.displayEstimate && this.displayEstimate() |
There was a problem hiding this comment.
Nit: it is unusual for constructors to have side-effects (in this case, printing something). I would make this behavior more explicit and require callers to call this method explicitly
There was a problem hiding this comment.
Great feedback, I had it that way originally and I'll change it back
| const progress = Math.round((durationInMs / this.estimateInMs) * 100) | ||
|
|
||
| this.progressBar.update(progress, { | ||
| estimate: TimeUtils.renderSpan(timeRemaining, { hideMilliseconds: true }), |
There was a problem hiding this comment.
If timeRemaining is negative, this will show a negative estimate. Is there a way to show "unknown" or maybe "soon" instead?
There was a problem hiding this comment.
The behavior currently involves "counting up" after the countdown is complete. I talked through some options with Jason when initially implementing this and this seemed like the best one. This PR is only a refactor, so I'll leave as is but I'll continue to think through more options.
Summary
Adds a transaction timer class that abstracts the timer logic away from combine notes.
This is in setup for adding the timer to
wallet:sendTesting Plan
Documentation
Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference)? If yes, link a
related documentation pull request for the website.
Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and label it with
breaking-change-rpcorbreaking-change-sdk.