Reserve greater than dust #1220
Conversation
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 |
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.
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.
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...
|
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 <rusty@rustcorp.com.au>
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 <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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 <rusty@rustcorp.com.au>
'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 <rusty@rustcorp.com.au>
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 <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
reserve-greater-than-dust
branch
from
2222855 to
7682f78
Compare
|
Removed WIP; this is a good change indep. of what happens to spec. |
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ound `channel_announcement` handling (ElementsProject#1220)" Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is an attempt to implement the lightning/bolts/pull/389 requirements, which should prevent us from ever producing all-dust outputs.