BOLT catchup, including dual funding! #7086
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustyrussell
force-pushed
the
guilt/bolt-catchup
branch
2 times, most recently
from
fea348f to
a28ab21
Compare
rustyrussell
force-pushed
the
guilt/bolt-catchup
branch
2 times, most recently
from
77d1fc8 to
1180e72
Compare
rustyrussell
force-pushed
the
guilt/bolt-catchup
branch
2 times, most recently
from
fcbf472 to
6705805
Compare
There was a problem hiding this comment.
CI catch something that I was able to reproduce also on my host
-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================================================================================================================= short test summary info ========================================================================================================================
FAILED tests/test_opening.py::test_rbf_non_last_mined - AssertionError: assert 'f8ff15a3f0d2...87a6a12f4bfe8' == 'f5fecb7b2b00...bd148b580a23e'
No it was removed in favor of no restrictions on witness weight payments! |
Does code need to change, or just comments? |
ShahanaFarooqui
force-pushed
the
guilt/bolt-catchup
branch
2 times, most recently
from
0214d1d to
8b043e0
Compare
|
I tried doing the obvious thing, removing the minimum witness weight requirement as it was removed from the spec. Correct me if I'm wrong @niftynei! |
endothermicdev
approved these changes
May 8, 2024
There was a problem hiding this comment.
Okay, I see how directly quoting the spec is basically a superpower.
ACK b60c512
| */ | ||
| next_feerate = last_feerate * 65 / 64; | ||
| next_feerate = last_feerate * 25 / 24; |
There was a problem hiding this comment.
Still kind of weird to round down a >=, but it's nice that the spec is explicit.
| */ | ||
| u32 next_min = last_feerate * 65 / 64; | ||
| u32 next_min = last_feerate * 25 / 24; |
There was a problem hiding this comment.
And here it bites us. The spec should be updated with the "round down" verbiage here too.
1. onion-message 2. blinded-payments 3. route-blinding 4. channel-type 5. warnings. Now they'll be checked correctly, and if the spec changes, we'll know to reexamine this code. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
And deprecate the --max-locktime-blocks which allows them to set it. Hilariously, the spec misspells CLTV as CTLV at one point, so we work around it for now. Signed-off-by: Rusty Russell <[email protected]>
Mainly we don't need to patch the spec anywhere near as much. Signed-off-by: Rusty Russell <[email protected]>
Merged spec says: ``` | 28/29 | `option_dual_fund` | Use v2 of channel open, enables dual funding | IN | ``` Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
Seems like the final spec got a bit stricter. Signed-off-by: Rusty Russell <[email protected]>
Spec says exactly what to test. Signed-off-by: Rusty Russell <[email protected]>
I can't find such a quote, and I'm not sure if this is correct? Signed-off-by: Rusty Russell <[email protected]>
A few of them had minor wording changes, too. Signed-off-by: Rusty Russell <[email protected]>
Also fix move on uninitialized value
removing min-witness-weight requirement
Removing the min_witness_weight saves the lessor 6 sats. lessor's DF tx vout: "value": 4.99492248 -> "value": 4.99492254
A race condition seemed to be redirecting rpc before bitcoind processed the second rbf transaction.
endothermicdev
force-pushed
the
guilt/bolt-catchup
branch
from
b60c512 to
bb95de4
Compare
|
Rebased on master with renepay refactor. |
This test was failing CI under valgrind. Allow l1 to process incoming signatures before killing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@niftynei I can't find one quote: does the 110 minimum witness weight thing still apply? See the second-last commit...