aljaz/lightning
1
0
Fork 0
mirror of https://github.com/aljazceru/lightning.git synced 2025-12-19 07:04:22 +01:00
Code Issues Packages Projects Releases Wiki Activity

lightningd: apply feerate changes correctly.

Browse Source 
Feerate changes are asymmetric, as they can only be sent by the funder.

For FUNDER, the remote feerate is set when upon send of
commitment_signed, and the local feerate is set on receipt of
revoke_and_ack.

For non-funder, the local feerate is set on receipt of
commitment_signed, and the remote feerate set on send of
revoke_and_ack.  In our code, these two happen together.

channeld gets this right, but lightningd ignored the funder/fundee
distinction, and as a result, receipt of a commitment_signed by the
funder altered fees in the database.  If there was a reconnection
event or restart, then these (incorrect) values would be used, causing
us to complain about a 'Bad commit_sig signature' and close the
channel.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell
2019-10-28 14:03:44 +10:30
committed by neil saitug
parent 61e1d6431c
commit 21d2cc663b
3 changed files with 15 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@@ -59,6 +59,7 @@ Note: You should always set `allow-deprecated-apis=false` to test for changes.
### Fixed
- Fixed bogus "Bad commit_sig signature" which caused channel closures when reconnecting after updating fees under simultaneous bidirectional traffic.
- Relative `--lightning_dir` is now working again.
- Build: MacOS now builds again (missing pwritev).

24
lightningd/peer_htlcs.c
View File

@@ -1343,8 +1343,10 @@ void peer_sending_commitsig(struct channel *channel, const u8 *msg)
channel->next_htlc_id += num_local_added;
}
/* Update their feerate. */
channel->channel_info.feerate_per_kw[REMOTE] = feerate;
/* Update remote feerate if we are funder. */
if (channel->funder == LOCAL)
channel->channel_info.feerate_per_kw[REMOTE] = feerate;
if (feerate > channel->max_possible_feerate)
channel->max_possible_feerate = feerate;
if (feerate < channel->min_possible_feerate)
@@ -1546,12 +1548,13 @@ void peer_got_commitsig(struct channel *channel, const u8 *msg)
}
}
/* Update both feerates: if we're funder, REMOTE should already be
* that feerate, if we're not, we're about to ACK anyway. */
channel->channel_info.feerate_per_kw[LOCAL]
= channel->channel_info.feerate_per_kw[REMOTE]
= feerate;
/* Update both feerates if we're not funder (for funder, receiving
* commitment_signed doesn't alter fees). */
if (channel->funder == REMOTE) {
channel->channel_info.feerate_per_kw[LOCAL]
= channel->channel_info.feerate_per_kw[REMOTE]
= feerate;
}
if (feerate > channel->max_possible_feerate)
channel->max_possible_feerate = feerate;
if (feerate < channel->min_possible_feerate)
@@ -1658,9 +1661,10 @@ void peer_got_revoke(struct channel *channel, const u8 *msg)
return;
}
/* Update feerate: if we are funder, their revoke_and_ack has set
/* Update feerate if we are funder, their revoke_and_ack has set
* this for local feerate. */
channel->channel_info.feerate_per_kw[LOCAL] = feerate;
if (channel->funder == LOCAL)
channel->channel_info.feerate_per_kw[LOCAL] = feerate;
/* FIXME: Check per_commitment_secret -> per_commit_point */
update_per_commit_point(channel, &next_per_commitment_point);

1
tests/test_connection.py
View File

@@ -2148,7 +2148,6 @@ def test_feerate_spam(node_factory, chainparams):
l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE', timeout=5)
@pytest.mark.xfail(strict=True)
@unittest.skipIf(not DEVELOPER, "need dev-feerate")
def test_feerate_stress(node_factory, executor):
# Third node makes HTLC traffic less predictable.