From rusty at rustcorp.com.au  Mon Apr  4 02:04:34 2016
From: rusty at rustcorp.com.au (Rusty Russell)
Date: Mon, 04 Apr 2016 11:34:34 +0930
Subject: [Lightning-dev] Acknowledgements in BOLT #2
In-Reply-To: <CAMXsxZNvbbVMkqyhTsKXjx+W9NxAFapGegfgCCH2jiAHLr5oiA@mail.gmail.com>
References: <87poua8ayh.fsf@rustcorp.com.au>
	<CAMXsxZNvbbVMkqyhTsKXjx+W9NxAFapGegfgCCH2jiAHLr5oiA@mail.gmail.com>
Message-ID: <87a8la86z1.fsf@rustcorp.com.au>

Pierre <pm+lists at acinq.fr> writes:
> Hello all,
>
> I noticed in the meantime Rusty recently introduced htlc ids in the
> form of absolute "lnd-like 64 bits unique id" vs the previous relative
> "order-I-added-them-in" id. I like it better and I think this is
> closely related to the issue at hand.

Yes.  I wanted to avoid it (it's YA index to keep, to map these 64 bit
IDs to the HTLC), but we can't: we really need to distinguish two HTLCs
with the same R value.  This is because if we don't allow that, it's
trivial for an attacker to probe us to find out if we've seen an HTLC.

Also, if we switch to keypair-based contracts, it's one less change to
make.

>> This is used so we know what the other side has received when they send
>>an "update_commit" message, and so we know where to restart the
>>conversation after reconnect ("authenticate" message).
>
> To me those two cases should be handled differently because they do
> not happen at the same level:
> - at the protocol level I would only use the htlc id. It is currently
> already used in add/fulfill/fail, so it would make sense to have it in
> the update_commit msg as well, instead of relying on the acknowledge
> field

You can't use this for (as Fabrice pointed out) because there are
multiple HTLCs you may be acknowledging, and you may be acknowledging
their removal.  Also, you may be acking a fee change.  In future this
may be changes to the anchor, as well.

So it really is simplest as "number of messages I've processed".

> - at the transport level (including the reconnect scenario) I would
> rely on the acknowledge field to know which messages should be
> replayed
>
> Yes it is kind of redundant, and probably less optimal, but it does
> separate clearly the transport from the protocol and testing might be
> easier.

They're actually closely tied.  Every time you send an 'ack' (that's in
an update_commit or update_revocation), you *must* have committed state
to persistent storage.  This matches nicely ie. you want to make sure
you remember if you've ever sent a revocation preimage, and you must
remember transactions you've signed.

> Also @lnd guys: what's the thing with odd/even htlc ids? Is it just so
> that we can use the same keyspace for incoming/outgoing htlcs and
> quickly tell the direction of a given htlc, or is there something
> else?

I really like this differentiation to catch bugs.  The latest spec
leaves this open, but my implementation currently uses:

	/* Make it different from other node (to catch bugs!), but a
	 * round number for simple eyeballing. */
	peer->htlc_id_counter = pseudorand(1ULL << 32) * 1000;

Cheers,
Rusty.