Description
Filter::register_tx documents its script_pubkey as "provided for informational purposes and may be useful for block sources which only support filtering on scripts", and states that the transaction has an output with it.
ElectrumSyncClient discards it (fn register_tx(&self, txid: &Txid, _script_pubkey: &Script)) and, in get_confirmed_transactions, derives the script to query from tx.output.first() instead, "an arbitrary output", as the comment there says.
If that first output is one the server returns no script history for, the lookup comes back empty and the transaction is never reported as confirmed. For a channel funding transaction this means the channel never reaches channel_ready.
Why this is reachable
LDK does not build funding transactions, the user does, in response to the FundingGenerationReady event. batch_funding_transaction_generated_intern validates only that inputs are segwit, that there are at most 2^16 outputs, and that the locktime is final; it places no restriction on the output scripts. So a wallet is free to put a data output first.
We hit this with RGB, which commits to its state in an OP_RETURN placed in the first output of every transaction. Electrum servers don't index provably unspendable outputs, so the history is empty.
The esplora client is unaffected, as it resolves transactions by txid.
BDK had the same first-output assumption and fixed it in bitcoindevkit/bdk#2195.
Suggested fix
Store the script_pubkey in FilterQueue/SyncState and query its history, instead of deriving one from the transaction. This is what the trait already promises: the script belongs to an output of that transaction, and for a channel funding transaction it's the funding output's, which is always indexable.
The script has to outlive confirmation. sync_confirmed_transactions removes the txid from watched_transactions, but a reorg re-adds it in
sync_unconfirmed_transactions, which is given bare txids (from Confirm::get_relevant_txids) and never calls register_tx again. Dropping the script on confirmation would leave such a transaction unlookupable, so it could never re-confirm.
I have a patch against main doing this, plus a test_syncing! case covering a transaction whose first output is an OP_RETURN (it fails before the change and passes after). I can't open a PR, but I'm happy to paste the diff here or in whatever form is easiest.
Description
Filter::register_txdocuments itsscript_pubkeyas "provided for informational purposes and may be useful for block sources which only support filtering on scripts", and states that the transaction has an output with it.ElectrumSyncClientdiscards it (fn register_tx(&self, txid: &Txid, _script_pubkey: &Script)) and, inget_confirmed_transactions, derives the script to query fromtx.output.first()instead, "an arbitrary output", as the comment there says.If that first output is one the server returns no script history for, the lookup comes back empty and the transaction is never reported as confirmed. For a channel funding transaction this means the channel never reaches
channel_ready.Why this is reachable
LDK does not build funding transactions, the user does, in response to the
FundingGenerationReadyevent.batch_funding_transaction_generated_internvalidates only that inputs are segwit, that there are at most 2^16 outputs, and that the locktime is final; it places no restriction on the output scripts. So a wallet is free to put a data output first.We hit this with RGB, which commits to its state in an
OP_RETURNplaced in the first output of every transaction. Electrum servers don't index provably unspendable outputs, so the history is empty.The esplora client is unaffected, as it resolves transactions by txid.
BDK had the same first-output assumption and fixed it in bitcoindevkit/bdk#2195.
Suggested fix
Store the
script_pubkeyinFilterQueue/SyncStateand query its history, instead of deriving one from the transaction. This is what the trait already promises: the script belongs to an output of that transaction, and for a channel funding transaction it's the funding output's, which is always indexable.The script has to outlive confirmation.
sync_confirmed_transactionsremoves the txid fromwatched_transactions, but a reorg re-adds it insync_unconfirmed_transactions, which is given bare txids (fromConfirm::get_relevant_txids) and never callsregister_txagain. Dropping the script on confirmation would leave such a transaction unlookupable, so it could never re-confirm.I have a patch against
maindoing this, plus atest_syncing!case covering a transaction whose first output is anOP_RETURN(it fails before the change and passes after). I can't open a PR, but I'm happy to paste the diff here or in whatever form is easiest.