client/asset/btc: work around PublishTransaction delays #1376
Conversation
chappjc
force-pushed
the
btc-spv-send-fast
branch
from
350109d
to
9bc8f02
Compare
|
The upstream fix is unlikely to happen this week, and even if it did, we would definitely not want broadcast to consistently block for even 4 seconds, which appears to be most common even when all peers to do respond with In parallel, I'm looking to make certain requests async, with particular focus on not holding up preimage and match_proof requests. |
chappjc
force-pushed
the
btc-spv-send-fast
branch
from
9bc8f02
to
dc5c072
Compare
| // rebroadcasting. In the future, a new btcwallet method should be added | ||
| // that returns after performing its internal actions, but broadcasting | ||
| // asynchronously and sending the outcome in a channel or promise. |
There was a problem hiding this comment.
I hate the guesstimation in this PR using 2 seconds to let it process and queue the txn when 100 ms is more than adequate most of the time, but the proper solution is described on these lines (a different btcwallet method).
| // because if an external bug, but even if the resolved, a typical | ||
| // inv/getdata round trip is ~4 seconds, so we set this so neutrino does | ||
| // not cancel queries too readily. | ||
| BroadcastTimeout: 6 * time.Second, |
There was a problem hiding this comment.
Looks like the default is 5? So we're actually giving neutrino more time, just not waiting around anymore.
There was a problem hiding this comment.
Correct. So 10 or even 30 would be fine with this approach, but I figured I would set it to something based on my empirical observations now that I've studied it more in practice. 5 seemed just under the wire to me.
|
We will definitely see a follow up to this in the future in the form of a neutrino update and/or a new btcwallet method that provides partially asynchronous PublishTransaction functionality without waiting for peer responses, only tx recording and queuing. |
Because of (a) what appears to be a neutrino bug, with a fix proposed in lightninglabs/neutrino#240, and (b) that neutrino
wallet.PublishTransactionis blocking whileinvrequests go to peers andgetdataresponses come back, this PR attempts to avoid an unnecessary delay with BTC SPV when broadcasting transactions.In all my observations,
PublishTransactionuses the entireneutrino.Config.BroadcastTimeout, which is nearly an intolerable delay for many operations.With neutrino as the wallet's chain service, its
chainClient.SendRawTransactioncall is blocking for up toneutrino.Config.BroadcastTimeoutwhile peers either respond to theinvrequest with agetdata, or time out. However, in virtually all cases, we just need to know that btcwallet was able to create and store the transaction record, and pass it to the chain service. This is a yucky workaround to consider until lightninglabs/neutrino#240 is accepted, maybe even complementary to it.