UTXO chain keysign failure messages are not being broadcast
Keysign failure messages can be seen by for instance
https://thornode.ninerealms.com/txs?limit=100&message.action=set_tss_keysign_fail&tx.height=6950224
or
https://viewblock.io/thorchain/txs?type=tss
('tssKeySignFail').
I have never seen a BTC chain failure message (or one of the other three UTXO chains, BCH/LTC/DOGE), and previously thought they were less frequent.
I was recently looking at a Bifrost log in which four outbounds, with 'Signing transaction' at the same, time, all had keysign failures ('didn't receive signature after 1m0s' at the same time). They were BTC/GAIA/BNB/BCH, and only the GAIA and BNB cases had "post keysign failure to thorchain".
Taking the BTC case as an example, it had
'fail to sign the message: fail to get witness: cannot sign tx input: fail to tss sign: fail to complete TSS keysign,reason:signers fail to sync before keygen/keysign'
.
The 'fail to get witness' was from here, showing that the WitnessSignature error was non-nil, but there was no "fail to post keysign failure to thorchain" or "post keysign failure to thorchain" log for it, showing that errors.As(err, &keysignError) returned false, not true.
Though I briefly went down a garden path,
my current impression is this:
In Errorf, the distinction between %s (to include something as a string) and %w (to wrap an error) is significant. Namely, %w must be used if errors.As is to be able to unwrap it, as with the Binance and Gaia cases.
Note for instance return fmt.Errorf("fail to get witness: %w", err), which uses %w.
The err checked by errors.As (when non-nil) comes from WitnessSignature.
WitnessSignature in turn only returns an error if it gets that error from RawTxInWitnessSignature.
RawTxInWitnessSignature gets its error from signature, err := signable.Sign(hash), BUT returns it in the form
return nil, fmt.Errorf("cannot sign tx input: %s", err).
I currently posit that this %s prevents the Sign-returned error from being further unwrapped (causing errors.As to always return false).
For more complete context:
BTC and LTC call WitnessSignature.
LTC refers to a different repository which similarly uses an %s.
BCH (noted to similarly not broadcast a keysign failure) calls RawTxInECDSASignature, which has return nil, fmt.Errorf("cannot sign tx input: %s", err).
DOGE uses RawTxInSignature, which similarly uses an %s.
I am out of time for the moment, but further consideration: DOGE's RawTxInSignature can for instance return an error from CalcSignatureHash, which uses %v. A thorough check of all errors that can be returned by these repositories' functions, and whether they should use %w, may thus be appropriate.
Additional comments for the moment:
The Sign called by BTC's RawTxInWitnessSignature is the function that produces this 'msg to sign:' log early on.
Any error from it comes from RemoteSign, which appears to be called by many or all chains, and which uses %w to wrap a non-nil error from a task's response.
Update:
Merge Requests have now been made for the four UTXO TxScript repositories.
thorchain/bifrost/txscript!4
thorchain/bifrost/bchd-txscript!2
thorchain/bifrost/ltcd-txscript!2
thorchain/bifrost/dogd-txscript!1
Edit:
If the above four Merge Requests are merged, this THORNode repository will also (I believe) require a Merge Request to update of the 'txscript'-containing lines of go.mod and go.sum.
At present I believe I am able to write this, using the commit SHA and commit time and sum.golang.org once all four repositories' Merge Requests have been committed.