Add a test parsing transaction with a huge witness#1359
Add a test parsing transaction with a huge witness#1359sanket1729 merged 1 commit intorust-bitcoin:masterfrom
Conversation
This transaction broke past versions of `rust-bitcoin` and LND so this adds a test to avoid reintroducing the problem in the future. See also romanz/electrs#783
|
IIUC, the failure is due to and indeed the witness definition has changed between 0.27.1 and 0.28.0: diff --git a/src/blockdata/transaction.rs b/src/blockdata/transaction.rs
index 4ecac18..445da7d 100644
--- a/src/blockdata/transaction.rs
+++ b/src/blockdata/transaction.rs
@@ -196,7 +198,7 @@ pub struct TxIn {
/// Encodable/Decodable, as it is (de)serialized at the end of the full
/// Transaction. It *is* (de)serialized with the rest of the TxIn in other
/// (de)serialization routines.
- pub witness: Vec<Vec<u8>>
+ pub witness: Witness
}The fix was introduced in 2fd0125. |
|
Damn, ideally this kind of thing would have been addressed through #1023. |
|
Ah, this is about rust-bitcoin 0.28, the above landed in 0.29 (according to github). So hopefully we have a substantially more robust answer to these kinds of bugs in 0.29 already. |
|
AFAIU even 0.28 is unaffected. This PR just adds test out of caution. |
elichai
left a comment
There was a problem hiding this comment.
ACK, this is an important test, as this tx did break production systems
|
I could not foresee the impact when I opened #997. It also lists other potential ways in which block decoding could fail, but all of them required the miner to specifically craft a non-standard tx. Perhaps, this is a lesson for us for back-porting all bug fixes related to decoding/encoding even if they "require miner assistance to deploy". |
|
I have created #1360 as a backport for 0.28 users. |
a0489d4 fuzz: use travis-fuzz.sh in CI (Andrew Poelstra) 4c6f9b3 fuzz: remove mysteriously-not-necessary quotes from gh action script (Andrew Poelstra) 7baa21c fuzz: disable features in honggfuzz (Andrew Poelstra) e003e57 Add `consensus_decode_from_finite_reader` optimization (Dawid Ciężarkiewicz) Pull request description: Backport for #1023. Required for #997. Addresses issues like #1359 ACKs for top commit: tcharding: ACK a0489d4 TheBlueMatt: ACK a0489d4. Tree-SHA512: 9145d9666e35ae77598aaecf89222c7234637b57ded39b69fbb93ee9ce01c6d7c938b36a2d86319ba84155f2424e524386593d6c0d7af449be6bd118f729fd64
This transaction broke past versions of
rust-bitcoinand LND so this adds a test to avoid reintroducing the problem in the future.See also romanz/electrs#783
I'm publishing this immediately for research purposes. I can clean it up later if required (low on time rn) or we may even not merge it if there is a better test.