Fix spurious panic on bogus funding txn that confirm and are spent by TheBlueMatt · Pull Request #1585 · lightningdevkit/rust-lightning

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt · 2022-07-01T14:19:12Z

In c02b6a3 we moved the
payment_preimage copy from inside the macro which only runs if we
are spending an output we know is an HTLC output to doing it for
any script that matches our expected length. This can panic if an
inbound channel is created with a bogus funding transaction that
has a witness program of the HTLC-Success/-Offered length but which
does not have a second-to-last witness element which is 32 bytes.

Luckily this panic is relatively simple for downstream users to
work around - if an invalid-length-copy panic occurs, simply remove
the ChannelMonitor from the bogus channel on startup and run
without it. Because the channel must be funded by a bogus script in
order to reach this panic, the channel will already have closed by
the time the funding transaction is spent, and there can be no
local funds in such a channel, so removing the ChannelMonitor
wholesale is completely safe.

In order to test this we have to disable an in-line assertion that
checks that our transactions match expected scripts which we do by
checking for the specific bogus script that we now use in
test_invalid_funding_tx.

Thanks to Eugene Siegel for reporting this issue.

ariard

Code Review ACK 9d91dac, in conformity to what have been reviewed elsewhere.

TheBlueMatt · 2022-07-01T14:30:19Z

Oops, had some rebase errors in use statements, had to resolve that.

ariard

Code Review ACK dfc6a32

Changes since last push :

removing the use for Transaction, TxOut, TxIn, OutPoint as BitcoinOutPoint in functional tests
removing the use for Witness in functional tests

In c02b6a3 we moved the `payment_preimage` copy from inside the macro which only runs if we are spending an output we know is an HTLC output to doing it for any script that matches our expected length. This can panic if an inbound channel is created with a bogus funding transaction that has a witness program of the HTLC-Success/-Offered length but which does not have a second-to-last witness element which is 32 bytes. Luckily this panic is relatively simple for downstream users to work around - if an invalid-length-copy panic occurs, simply remove the ChannelMonitor from the bogus channel on startup and run without it. Because the channel must be funded by a bogus script in order to reach this panic, the channel will already have closed by the time the funding transaction is spent, and there can be no local funds in such a channel, so removing the `ChannelMonitor` wholesale is completely safe. In order to test this we have to disable an in-line assertion that checks that our transactions match expected scripts which we do by checking for the specific bogus script that we now use in `test_invalid_funding_tx`. Thanks to Eugene Siegel for reporting this issue.

TheBlueMatt · 2022-07-01T14:49:56Z

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

ariard

One More Time Code Review ACK 6c480ae 🎵

Oops, sorry, one more, looks like Vec::from(array) needed const generics which we don't support.

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

TheBlueMatt · 2022-07-01T15:01:57Z

Hmmm, the previous branch built well on my local host. Some issue with bindings or other platforms than x86_64 ?

It was an MSRV issue.

dunxen

ACK 6c480ae

dunxen · 2022-07-01T15:42:55Z

+// in-line tests later.
+
+#[cfg(test)]
+pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {


Looks all bogus to me, but perhaps the beginning of a new age techno rap.

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

TheBlueMatt added this to the 0.0.109 milestone Jul 1, 2022

ariard previously approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt mentioned this pull request Jul 1, 2022
Cut 0.0.109 #1582
Merged

TheBlueMatt dismissed ariard’s stale review via 1f80fadJuly 1, 2022 14:28

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch 2 times, most recently from 1f80fad to dfc6a32Compare July 1, 2022 14:30

ariard reviewed Jul 1, 2022
View reviewed changes

TheBlueMatt force-pushed the 2022-06-copy_from_slice-sucks branch from dfc6a32 to 6c480aeCompare July 1, 2022 14:47

ariard approved these changes Jul 1, 2022
View reviewed changes

dunxen approved these changes Jul 1, 2022
View reviewed changes

jkczyz approved these changes Jul 1, 2022
View reviewed changes

TheBlueMatt merged commit d9ba7ce into lightningdevkit:mainJul 1, 2022

Crypt-iQ mentioned this pull request Jul 17, 2023
[7/?] - funding: update funding manager w/ new musig2+taproot funding flow lightningnetwork/lnd#7346
Merged
7 tasks

Uh oh!

Conversation

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

dunxen left a comment

Choose a reason for hiding this comment

Uh oh!

dunxenJul 1, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

dunxen left a comment

Choose a reason for hiding this comment

Uh oh!

dunxenJul 1, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!

ariard left a comment

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jul 1, 2022

Uh oh!