Reserve greater than dust#1220
Merged
cdecker merged 6 commits intoElementsProject:masterfrom Apr 5, 2018
Merged
Conversation
cdecker
reviewed
Mar 16, 2018
Member
cdecker
left a comment
cdecker
left a comment
There was a problem hiding this comment.
Looking good so far. The first commit also needs to bump the funding amount on the following tests for it to pass CI:
test_bech32_fundingtest_disconnect_fundeetest_disconnect_fundertest_disconnect_half_signedtest_reconnect_openingdtest_reconnect_signedtest_routing_gossiptest_routing_gossip_reconnect
Alternatively we can bump the reserve required to be 2.5x the current value.
| * - MUST set `channel_reserve_satoshis` greater than or equal to | ||
| * `dust_limit_satoshis`. | ||
| */ | ||
| if (state->localconf.channel_reserve_satoshis |
Member
There was a problem hiding this comment.
This test should probably be extracted into a common function, it's being used twice. Also there is the asymmetry of one calling peer_failed and the other calling status_failed.
Contributor
Author
There was a problem hiding this comment.
I'm going to rework this so we get clearer messages: for the local side we want to say "we refused to offer xxx" or "they offered xxx" and slight language differences for the error message we send to peer.
Contributor
Author
There was a problem hiding this comment.
OK, I've switched this to adjust the reserve to meet the minimum.
I also added a patch which make negotiation_failed clearer: it's for use when they give us something we don't agree with, not when we set parameters we can't meet.
Am testing to see what tests this broke...
Contributor
Author
|
I wonder if we should just set reserve to 1% or dust limit, whatever is greater. |
This quotes from the BOLT proposal at lightning/bolts#389 Don't try to fund channels with reserve less than dust, nor allow them to fund channels with reserve less than dust. Fixes: ElementsProject#632 Signed-off-by: Rusty Russell <[email protected]>
This quotes from the BOLT proposal at lightning/bolts#389 Don't try to fund channels which would do this, and don't allow others to fund channels which would do this. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
This is probably covered by our "channel capacity" heuristic which requires the channel be significant, but best to be explicit and sure. Signed-off-by: Rusty Russell <[email protected]>
'negotiation_failed' is currently just a useless wrapper around peer_failed (a vestige from when peer_failed would close the connection). Change it to send different local and remote messages, and use it wherever we dislike their parameters: stick with peer_failed if we dislike our own parameters. Signed-off-by: Rusty Russell <[email protected]>
I saw a failure in test_funding_fail():
assert l2.rpc.listpeers()['peers'][0]['connected']
This can happen if l2 hasn't yet handed back to gossipd. Turns out
we didn't mark uncommitted channels as connected:
[{'id': '03afa3c78bb39217feb8aac308852e6383d59409839c2b91955b2d992421f4a41e', 'connected': False, 'channels': [{'state': 'OPENINGD', 'owner': 'lightning_openingd', 'funder': 'REMOTE', 'status': ['Incoming channel: accepted, now waiting for them to create funding tx']}]}]
Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
force-pushed
the
reserve-greater-than-dust
branch
from
2222855 to
7682f78
Compare
Contributor
Author
|
Removed WIP; this is a good change indep. of what happens to spec. |
cdecker
approved these changes
Apr 5, 2018
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Feb 26, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 4, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 4, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 12, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 15, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 15, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
to rustyrussell/lightning
that referenced
this pull request
Mar 15, 2025
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <[email protected]>
rustyrussell
added a commit
that referenced
this pull request
Mar 18, 2025
…ound `channel_announcement` handling (#1220)" Signed-off-by: Rusty Russell <[email protected]>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This is an attempt to implement the lightning/bolts/pull/389 requirements, which should prevent us from ever producing all-dust outputs.