Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Let redemption auto submit quit if not enough confirmations #112

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nkuba
Copy link
Member

@nkuba nkuba commented Oct 29, 2020

Previously the autoSubmit function hanged waiting for bitcoin transactions confirmations. With this change we are adding a possibility to quit the flow in case the confirmations number is insufficient.

Previously the function hanged waiting for bitcoin transactions
confirmations. With this change we are adding a possibility to quit the
flow in case the confirmations number is insufficient.
@nkuba nkuba requested a review from Shadowfiend October 29, 2020 15:32
*/
checkConfirmations: async function(transactionID, requiredConfirmations) {
return BitcoinHelpers.withElectrumClient(async electrumClient => {
// TODO: Consider using retry backoff to get transaction.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100%, have this in another branch that improves our retries heh.

Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually question this move. The whole point of autoSubmit is that it runs through the whole submission flow end to end.

I think this requirement is best expressed as a new function of some sort, though I acknowledge that'll probably require some refactoring.

I'm guessing the goal here is when one is trying to model “submit a funding proof if possible” as a flow. To me this means the work broadcastTransactionID does in autoSubmit should be put in its own function, which can be called by an outside caller if they want to get the transaction id. Then they can check confirmations directly, and autoSubmit can continue to operate autonomously until completion when called. Thoughts?

@Shadowfiend
Copy link
Contributor

Going to hold this guy for now since it's not strictly necessary for tbtc-dapp and the script that is leveraging it is already using this version of the code.

@nkuba nkuba force-pushed the dont-wait-for-confirmations branch from 33d3864 to 662b2c7 Compare May 24, 2021 09:04
@nkuba nkuba force-pushed the dont-wait-for-confirmations branch from 662b2c7 to f554e6b Compare May 24, 2021 09:30
@nkuba nkuba marked this pull request as draft September 14, 2021 10:56
@nkuba nkuba force-pushed the dont-wait-for-confirmations branch from 5cc933f to c58b9ce Compare September 14, 2021 11:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants