daemon: commit outstanding changes via timer.

While useful for testing, it doesn't make sense to have an explicit commit
command; we should commit whenever there are outstanding changes.

We have a 10ms timer to allow limited batching, however.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2016-05-10 06:30:11 +09:30
parent fe1ba96332
commit 35d1b13cde
7 changed files with 123 additions and 52 deletions

View File

@ -192,7 +192,8 @@ daemon-test-%:
daemon-test-steal: daemon-test-dump-onchain
daemon-test-dump-onchain: daemon-test-timeout-anchor
daemon-test-timeout-anchor: daemon-test-normal
daemon-test-normal: daemon-all
daemon-test-normal: daemon-test-manual-commit
daemon-test-manual-commit: daemon-all
daemon-tests: daemon-test-steal

View File

@ -103,6 +103,9 @@ static void config_register_opts(struct lightningd_state *dstate)
opt_register_arg("--bitcoind-poll", opt_set_time, opt_show_time,
&dstate->config.poll_time,
"Time between polling for new transactions");
opt_register_arg("--commit-time", opt_set_time, opt_show_time,
&dstate->config.commit_time,
"Time after changes before sending out COMMIT");
}
#define MINUTES 60
@ -154,6 +157,9 @@ static void default_config(struct config *config)
config->max_expiry = 5 * DAYS;
config->poll_time = time_from_sec(30);
/* Send commit 10msec after receiving; almost immediately. */
config->commit_time = time_from_msec(10);
}
static void check_config(struct lightningd_state *dstate)

View File

@ -43,6 +43,9 @@ struct config {
/* How long between polling bitcoind. */
struct timerel poll_time;
/* How long between changing commit and sending COMMIT message.. */
struct timerel commit_time;
};
/* Here's where the global variables hide! */

View File

@ -217,7 +217,8 @@ void queue_pkt_htlc_add(struct peer *peer,
fatal("Could not add HTLC?");
peer_add_htlc_expiry(peer, &htlc_prog->stage.add.htlc.expiry);
their_commit_changed(peer);
queue_pkt_with_ack(peer, PKT__PKT_UPDATE_ADD_HTLC, u,
add_our_htlc_ourside,
tal_dup(peer, struct channel_htlc,
@ -248,6 +249,7 @@ void queue_pkt_htlc_fulfill(struct peer *peer,
/* We're about to send this, so their side will have it from now on. */
n = funding_htlc_by_id(&peer->them.staging_cstate->a, f->id);
funding_a_fulfill_htlc(peer->them.staging_cstate, n);
their_commit_changed(peer);
queue_pkt_with_ack(peer, PKT__PKT_UPDATE_FULFILL_HTLC, f,
fulfill_their_htlc_ourside, int2ptr(f->id));
@ -280,6 +282,7 @@ void queue_pkt_htlc_fail(struct peer *peer,
n = funding_htlc_by_id(&peer->them.staging_cstate->a, f->id);
funding_a_fail_htlc(peer->them.staging_cstate, n);
their_commit_changed(peer);
queue_pkt_with_ack(peer, PKT__PKT_UPDATE_FAIL_HTLC, f,
fail_their_htlc_ourside, int2ptr(f->id));
}
@ -605,6 +608,7 @@ Pkt *accept_pkt_htlc_add(struct peer *peer, const Pkt *pkt)
u->amount_msat);
peer_add_htlc_expiry(peer, &expiry);
their_commit_changed(peer);
/* FIXME: Fees must be sufficient. */
return NULL;
@ -651,6 +655,7 @@ Pkt *accept_pkt_htlc_fail(struct peer *peer, const Pkt *pkt)
funding_a_fail_htlc(peer->us.staging_cstate, n_us);
funding_b_fail_htlc(peer->them.staging_cstate, n_them);
their_commit_changed(peer);
return NULL;
}
@ -677,6 +682,7 @@ Pkt *accept_pkt_htlc_fulfill(struct peer *peer, const Pkt *pkt)
funding_a_fulfill_htlc(peer->us.staging_cstate, n_us);
funding_b_fulfill_htlc(peer->them.staging_cstate, n_them);
their_commit_changed(peer);
return NULL;
}

View File

@ -414,6 +414,45 @@ static void peer_disconnect(struct io_conn *conn, struct peer *peer)
}
}
static void do_commit(struct peer *peer, struct command *jsoncmd)
{
/* We can have changes we suggested, or changes they suggested. */
if (peer->them.staging_cstate
&& peer->them.commit
&& peer->them.commit->cstate
&& peer->them.staging_cstate->changes
!= peer->them.commit->cstate->changes) {
log_debug(peer->log, "do_commit: sending commit command");
set_current_command(peer, CMD_SEND_COMMIT, NULL, jsoncmd);
return;
}
log_debug(peer->log, "do_commit: no changes to commit");
if (jsoncmd) {
command_fail(jsoncmd, "no changes to commit");
return;
}
}
static void try_commit(struct peer *peer)
{
peer->commit_timer = NULL;
queue_cmd(peer, do_commit, (struct command *)NULL);
}
void their_commit_changed(struct peer *peer)
{
log_debug(peer->log, "their_commit_changed: changes=%u",
peer->them.staging_cstate->changes);
if (!peer->commit_timer) {
log_debug(peer->log, "their_commit_changed: adding timer");
peer->commit_timer = new_reltimer(peer->dstate, peer,
peer->dstate->config.commit_time,
try_commit, peer);
} else
log_debug(peer->log, "their_commit_changed: timer already exists");
}
static struct peer *new_peer(struct lightningd_state *dstate,
struct io_conn *conn,
int addr_type, int addr_protocol,
@ -452,6 +491,7 @@ static struct peer *new_peer(struct lightningd_state *dstate,
peer->closing_onchain.tx = NULL;
peer->closing_onchain.resolved = NULL;
peer->closing_onchain.ci = NULL;
peer->commit_timer = NULL;
/* Make it different from other node (to catch bugs!), but a
* round number for simple eyeballing. */
peer->htlc_id_counter = pseudorand(1ULL << 32) * 1000;
@ -488,6 +528,7 @@ static struct peer *new_peer(struct lightningd_state *dstate,
peer->log = new_log(peer, dstate->log_record, "%s%s:%s:",
log_prefix(dstate->base_log), in_or_out,
netaddr_name(peer, &peer->addr));
return peer;
}
@ -2370,17 +2411,6 @@ const struct json_command failhtlc_command = {
"Returns an empty result on success"
};
static void do_commit(struct peer *peer, struct command *jsoncmd)
{
/* We can have changes we suggested, or changes they suggested. */
if (peer->them.staging_cstate->changes == peer->them.commit->cstate->changes) {
command_fail(jsoncmd, "no changes to commit");
return;
}
set_current_command(peer, CMD_SEND_COMMIT, NULL, jsoncmd);
}
static void json_commit(struct command *cmd,
const char *buffer, const jsmntok_t *params)
{

View File

@ -212,6 +212,9 @@ struct peer {
/* Timeout for close_watch. */
struct oneshot *close_watch_timeout;
/* Timeout for collecting changes before sending commit. */
struct oneshot *commit_timer;
/* Private keys for dealing with this peer. */
struct peer_secrets *secrets;
@ -241,5 +244,8 @@ struct bitcoin_tx *peer_create_close_tx(struct peer *peer, u64 fee);
uint64_t commit_tx_fee(const struct bitcoin_tx *commit,
uint64_t anchor_satoshis);
/* We have something pending for them. */
void their_commit_changed(struct peer *peer);
bool resolve_one_htlc(struct peer *peer, u64 id, const struct sha256 *preimage);
#endif /* LIGHTNING_DAEMON_PEER_H */

View File

@ -55,6 +55,9 @@ while [ $# != 0 ]; do
x"--steal")
STEAL=1
;;
x"--manual-commit")
MANUALCOMMIT=1
;;
x"--normal")
;;
x"--verbose")
@ -93,17 +96,17 @@ lcli2()
$LCLI2 "$@"
}
# Usage: <node responsible> <cmd to test>...
# Usage: <cmd to test>...
check()
{
local i=0
local node="$1"
shift
while ! eval "$@"; do
# Try making time pass for the node (if on mocktime), then sleeping.
# Try making time pass for the nodes (if on mocktime), then sleeping.
if [ -n "$MOCKTIME" ]; then
MOCKTIME=$(($MOCKTIME + 1))
$node dev-mocktime $MOCKTIME
# Some tests kill nodes, so ignore failure here.
lcli1 dev-mocktime $MOCKTIME > /dev/null 2>&1 || true
lcli2 dev-mocktime $MOCKTIME > /dev/null 2>&1 || true
fi
sleep 1
i=$(($i + 1))
@ -123,7 +126,7 @@ check_status_single()
them_fee=$6
them_htlcs="$7"
if check $lcli "$lcli getpeers | tr -s '\012\011\" ' ' ' | $FGREP \"our_amount : $us_pay, our_fee : $us_fee, their_amount : $them_pay, their_fee : $them_fee, our_htlcs : [ $us_htlcs], their_htlcs : [ $them_htlcs]\""; then :; else
if check "$lcli getpeers | tr -s '\012\011\" ' ' ' | $FGREP \"our_amount : $us_pay, our_fee : $us_fee, their_amount : $them_pay, their_fee : $them_fee, our_htlcs : [ $us_htlcs], their_htlcs : [ $them_htlcs]\""; then :; else
echo Cannot find $lcli output: '"our_amount" : '$us_pay', "our_fee" : '$us_fee', "their_amount" : '$them_pay', "their_fee" : '$them_fee', "our_htlcs" : [ '"$us_htlcs"'], "their_htlcs" : [ '"$them_htlcs"']' >&2
$lcli getpeers | tr -s '\012\011 ' ' ' >&2
return 1
@ -148,7 +151,7 @@ check_staged()
lcli="$1"
num_htlcs="$2"
if check $lcli "$lcli getpeers | tr -s '\012\011\" ' ' ' | $FGREP 'staged_changes : '$num_htlcs"; then :; else
if check "$lcli getpeers | tr -s '\012\011\" ' ' ' | $FGREP 'staged_changes : '$num_htlcs"; then :; else
echo Cannot find $lcli output: '"staged_changes" : '$num_htlcs >&2
$lcli getpeers | tr -s '\012\011 ' ' ' >&2
return 1
@ -157,7 +160,7 @@ check_staged()
check_tx_spend()
{
if check "$1" "$CLI getrawmempool | $FGREP '\"'"; then :;
if check "$CLI getrawmempool | $FGREP '\"'"; then :;
else
echo "No tx in mempool:" >&2
$CLI getrawmempool >&2
@ -167,7 +170,7 @@ check_tx_spend()
check_peerstate()
{
if check "$1" "$1 getpeers | $FGREP -w $2"; then :
if check "$1 getpeers | $FGREP -w $2"; then :
else
echo "$1" not in state "$2": >&2
$1 getpeers >&2
@ -177,7 +180,7 @@ check_peerstate()
check_peerconnected()
{
if check "$1" "$1 getpeers | tr -s '\012\011\" ' ' ' | $FGREP -w 'connected : '$2"; then :
if check "$1 getpeers | tr -s '\012\011\" ' ' ' | $FGREP -w 'connected : '$2"; then :
else
echo "$1" not connected "$2": >&2
$1 getpeers >&2
@ -187,7 +190,7 @@ check_peerconnected()
check_no_peers()
{
if check "$1" "$1 getpeers | tr -s '\012\011\" ' ' ' | $FGREP 'peers : [ ]'"; then :
if check "$1 getpeers | tr -s '\012\011\" ' ' ' | $FGREP 'peers : [ ]'"; then :
else
echo "$1" still has peers: >&2
$1 getpeers >&2
@ -209,12 +212,20 @@ all_ok()
trap "echo Results in $DIR1 and $DIR2 >&2; cat $DIR1/errors $DIR2/errors >&2" EXIT
mkdir $DIR1 $DIR2
if [ -n "$MANUALCOMMIT" ]; then
# Aka. never.
COMMIT_TIME=1h
else
COMMIT_TIME=10ms
fi
cat > $DIR1/config <<EOF
log-level=debug
bitcoind-poll=1s
min-expiry=900
bitcoin-datadir=$DATADIR
locktime=600
commit-time=$COMMIT_TIME
EOF
cat > $DIR2/config <<EOF
@ -223,6 +234,7 @@ bitcoind-poll=1s
min-expiry=900
bitcoin-datadir=$DATADIR
locktime=600
commit-time=$COMMIT_TIME
EOF
if [ -n "$GDB1" ]; then
@ -239,12 +251,12 @@ else
$PREFIX ../lightningd --lightning-dir=$DIR2 > $REDIR2 2> $REDIRERR2 &
fi
if ! check "$LCLI1" "$LCLI1 getlog 2>/dev/null | $FGREP Hello"; then
if ! check "$LCLI1 getlog 2>/dev/null | $FGREP Hello"; then
echo Failed to start daemon 1 >&2
exit 1
fi
if ! check "$LCLI2" "$LCLI2 getlog 2>/dev/null | $FGREP Hello"; then
if ! check "$LCLI2 getlog 2>/dev/null | $FGREP Hello"; then
echo Failed to start daemon 2 >&2
exit 1
fi
@ -336,27 +348,34 @@ SECRET=1de08917a61cb2b62ed5937d38577f6a7bfe59c176781c6d8128018e8b5ccdfd
RHASH=`lcli1 dev-rhash $SECRET | sed 's/.*"\([0-9a-f]*\)".*/\1/'`
lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
# Nothing should have changed!
check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
# But 2 should register a staged htlc.
check_staged lcli2 1
if [ -n "$MANUALCOMMIT" ]; then
# Nothing should have changed!
check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
# But 2 should register a staged htlc.
check_staged lcli2 1
# Now commit it.
lcli1 commit $ID2
# Now commit it.
lcli1 commit $ID2
# Node 1 hasn't got it committed, but node2 should have told it to stage.
check_status_single lcli1 $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
check_staged lcli1 1
# Node 1 hasn't got it committed, but node2 should have told it to stage.
check_status_single lcli1 $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
check_staged lcli1 1
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
A_FEE=$(($A_FEE + $EXTRA_FEE))
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
A_FEE=$(($A_FEE + $EXTRA_FEE))
# Node 2 has it committed.
check_status_single lcli2 $B_AMOUNT $B_FEE "" $A_AMOUNT $A_FEE '{ "msatoshis" : '$HTLC_AMOUNT', "expiry" : { "second" : '$EXPIRY' }, "rhash" : "'$RHASH'" } '
# Node 2 has it committed.
check_status_single lcli2 $B_AMOUNT $B_FEE "" $A_AMOUNT $A_FEE '{ "msatoshis" : '$HTLC_AMOUNT', "expiry" : { "second" : '$EXPIRY' }, "rhash" : "'$RHASH'" } '
# Now node2 gives commitment to node1.
lcli2 commit $ID1
# Now node2 gives commitment to node1.
lcli2 commit $ID1
else
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
A_FEE=$(($A_FEE + $EXTRA_FEE))
fi
# Both should have committed tx.
check_status $A_AMOUNT $A_FEE '{ "msatoshis" : '$HTLC_AMOUNT', "expiry" : { "second" : '$EXPIRY' }, "rhash" : "'$RHASH'" } ' $B_AMOUNT $B_FEE ""
if [ -n "$STEAL" ]; then
@ -407,8 +426,8 @@ if [ -n "$DUMP_ONCHAIN" ]; then
fi
lcli2 fulfillhtlc $ID1 $SECRET
lcli2 commit $ID1
lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
# We've transferred the HTLC amount to 2, who now has to pay fees,
# so no net change for A who saves on fees.
@ -423,8 +442,8 @@ check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
HTLC_AMOUNT=100000000
lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
lcli1 commit $ID2
lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
@ -432,8 +451,8 @@ A_FEE=$(($A_FEE + $EXTRA_FEE))
check_status $A_AMOUNT $A_FEE '{ "msatoshis" : '$HTLC_AMOUNT', "expiry" : { "second" : '$EXPIRY' }, "rhash" : "'$RHASH'" } ' $B_AMOUNT $B_FEE ""
lcli2 failhtlc $ID1 $RHASH
lcli2 commit $ID1
lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
# Back to how we were before.
A_AMOUNT=$(($A_AMOUNT + $EXTRA_FEE + $HTLC_AMOUNT))
@ -443,8 +462,8 @@ check_status $A_AMOUNT $A_FEE "" $B_AMOUNT $B_FEE ""
# Same again, but this time it expires.
HTLC_AMOUNT=10000001
lcli1 newhtlc $ID2 $HTLC_AMOUNT $EXPIRY $RHASH
lcli1 commit $ID2
lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
# Check channel status
A_AMOUNT=$(($A_AMOUNT - $EXTRA_FEE - $HTLC_AMOUNT))
@ -458,8 +477,8 @@ lcli1 dev-mocktime $MOCKTIME
# This should make node2 send it.
MOCKTIME=$(($MOCKTIME + 31))
lcli2 dev-mocktime $MOCKTIME
lcli2 commit $ID1
lcli1 commit $ID2
[ ! -n "$MANUALCOMMIT" ] || lcli2 commit $ID1
[ ! -n "$MANUALCOMMIT" ] || lcli1 commit $ID2
# Back to how we were before.
A_AMOUNT=$(($A_AMOUNT + $EXTRA_FEE + $HTLC_AMOUNT))