Unparsable transactions block observation of parsable transactions in the same block
Discord thread:
https://discord.com/channels/838986635756044328/1193894402242912356/1196089452112658583
Most salient points:
https://blockexplorer.one/litecoin/mainnet/tx/c2fe754a57b4d5e144144097d4206be2eaff3f9d22e43a43833fdac25a056878
Transaction in LTC block 2608661 from ltc1qke3wlj559g5c58p5s65e8qvjkdt7y00kukd9hp
to Asgard vault ltc1qh7cjvuc3gtt3r4afm0zqvhvrpkfw0ahx845dsv.
( https://thornode-v1.ninerealms.com/thorchain/vault/thorpub1addwnpepqtsygya0axr2zxeqwj0d893qtrsunkxlnqtw7wv6yt6h4vmgqyz47jy37u9?height=14119457 )
https://blockexplorer.one/litecoin/mainnet/tx/e3f1e3daa2693fc46dba6be317f9044a32a2c38a9ae5e6da97af25b23f583e6b
Transaction in same LTC block 2608661 from LTC Taproot address (ltc1p) ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu
to Asgard vault ltc1qgpa0uz3ph4076j4f3kjxgzzd6t5298sd0wk2q4.
( https://thornode-v1.ninerealms.com/thorchain/vault/thorpub1addwnpepqvrwd5ggf5nx4q665qdrd9st76wjykadrzgh9v5tfq3f67jrwx5zkzdutvk?height=14119457 )
It is known that THORChain cannot parse Taproot addresses before the next hard fork;
however, this Issue is in regard to what happened to the ltc1q..d9hp inbound.
As can be seen from
https://midgard.ninerealms.com/v2/actions?address=ltc1qke3wlj559g5c58p5s65e8qvjkdt7y00kukd9hp
, THORChain has observed inbounds from ltc1q..d9hp at other times without problem. However:
https://thornode-v1.ninerealms.com/thorchain/tx/details/c2fe754a57b4d5e144144097d4206be2eaff3f9d22e43a43833fdac25a056878
Zero THORchain nodes observed this transaction.
In Bifrost logs of this period (intermediate lines not displayed):
"module":"blockscanner","chain":"LTC","block height":2608661,"txs":3
"gitlab.com/thorchain/thornode/bifrost/pkg/chainclients/litecoin/client.go:1163","message":"totalTxValue:50010000,total fee and Subsidy:628164765,confirmation:0"
"error":"address format not supported: ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu"[...]"caller":"gitlab.com/thorchain/thornode/bifrost/observer/observe.go:604","message":"fail to parse sender,ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu is invalid sender address"
Looking at that observe.go:604 origin:
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L578-580
func (o *Observer) getThorchainTxIns(txIn types.TxIn) (stypes.ObservedTxs, error) {
txs := make(stypes.ObservedTxs, 0, len(txIn.TxArray))
o.logger.Debug().Msgf("len %d", len(txIn.TxArray))
for _, item := range txIn.TxArray {[...]
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L611-617
sender, err := common.NewAddress(item.Sender)
if err != nil {
o.errCounter.WithLabelValues("fail_to_parse_sender", item.Sender).Inc()
// log the error , and ignore the transaction, since the address is not valid
o.logger.Err(err).Msgf("fail to parse sender,%s is invalid sender address", item.Sender)
return nil, nil
}[...]
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L647-649
}
return txs, nil
}This currently appears to be how one unparsable transaction can cause THORChain to fail to observe all the transactions in the same group (chunk? deck? block?).
My immediate impression is that every return within that range should instead be a continue, to not affect the observation of unrelated transactions in the same range.
Double-checking the context of getThorchainTxIns:
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L516-531
If getThorchainTxIns returns an error, signAndSendToThorchain returns an error;
if no error, signAndSendToThorchain turns the returned transactions into messages with GetObservationsStdTx,
then calls Broadcast for them.
(In practice, I believe broadcasting a single transaction with multiple MsgObservedTxIn messages;
as an example of such a broadcast,
https://thornode-v1.ninerealms.com/txs/7CD266D887295039C4C2901BE8732B819CD56D05F703EE12913F2DFBA126414D
.)
If getThorchainTxIns returns nil, nil then GetObservationsStdTx does the below:
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/thorclient/thorchain.go#L324-327
func (b *thorchainBridge) GetObservationsStdTx(txIns stypes.ObservedTxs) ([]cosmos.Msg, error) {
if len(txIns) == 0 {
return nil, nil
}after which signAndSendToThorchain has
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L524-526 \
if len(msgs) == 0 {
return nil
}signAndSendToThorchain is only called in chunkifyAndSendToThorchain;
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L225-231
for _, txIn := range o.chunkify(deck) {
if err := o.signAndSendToThorchain(txIn); err != nil {
o.logger.Error().Err(err).Msg("fail to send to THORChain")
// tx failed to be forward to THORChain will be added back to queue , and retry later
newTxIn.TxArray = append(newTxIn.TxArray, txIn.TxArray...)
continue
}In other words, if signAndSendToThorchain returns an error, then that chunk of transactions is retried over and over
(and the same unparsable transaction may block those with it from being broadcast every time).
However, if signAndSendToThorchain doesn't return an error, then the chunk of transactions is assumed to have been successfully observed and isn't retried.
Relevant is October 2023's merged !3173 (merged)
'Log and drop GetObservationsStdTx unobservable ObservedTx'
which similarly stopped a single problematic transaction from getting stuck in a loop by returning an error (and dragging its chunk along with it).
Also relevant is June 2023's !2950 (diffs)
'Ignore transaction when the sender address is invalid'
which I now see changed getThorchainTxIns 'fail to parse sender' behaviour
from 'erroring-and-retrying all the chunk's transactions'
to 'dropping all the chunk's transactions'.
I incidentally also note #1823 'All chain clients should have in/out transaction in log context'
which would have made finding the blocking transaction much easier: though
https://blockexplorer.one/litecoin/mainnet/tx/e3f1e3daa2693fc46dba6be317f9044a32a2c38a9ae5e6da97af25b23f583e6b
matches the ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu sender displayed in the Bifrost log,
I was initially trying to use Blockchair, which for the same transaction
https://blockchair.com/litecoin/transaction/e3f1e3daa2693fc46dba6be317f9044a32a2c38a9ae5e6da97af25b23f583e6b
displays the sender as ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqhry6w7
and shows no transactions for ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu .
https://blockchair.com/litecoin/address/ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqzl5ktu
(Nothing)
https://blockchair.com/litecoin/address/ltc1p4y6tas5457606tn67r3gtnjdv36e4cxat3q4xfnageuwnpxktzcqhry6w7
(Two transactions, including the block 2608661 one of note.)
Based on the above, I suggest that these transaction-specific returns in getThorchainTxIns
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/observer/observe.go#L606-630
(including an ObservedVaultPubKey.GetAddress error return)
and (for consistency) this transaction-specific error return in GetObservationsStdTx
https://gitlab.com/thorchain/thornode/-/blob/v1.126.0/bifrost/thorclient/thorchain.go#L338-341
obAddr, err := tx.ObservedPubKey.GetAddress(chain)
if err != nil {
return nil, err
}be changed to logs and continues, dropping and not observing just those unobservable transactions, while still observing other observable transactions in the same block.