Skip to content

Commit

Permalink
wire: Correct the short channel id serialization to use 3+3+2
Browse files Browse the repository at this point in the history
Fixes the `short_channel_id` being serialized as 4 bytes block height,
3 bytes transaction index and 1 byte output number, to use 3+3+2 as
the spec says.

The reordering in the unit test structs is mainly to be able to still
use `eq_upto` for tests.
  • Loading branch information
cdecker authored and rustyrussell committed May 20, 2017
1 parent b8ba8f0 commit 05e951d
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 17 deletions.
2 changes: 1 addition & 1 deletion daemon/routing.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ struct node_connection *get_connection_by_scid(const struct routing_state *rstat
num_conn = tal_count(n->out);
for (i = 0; i < num_conn; i++){
c = n->out[i];
if (structeq(&c->short_channel_id, schanid) &&
if (short_channel_id_eq(&c->short_channel_id, schanid) &&
(c->flags&0x1) == direction)
return c;
}
Expand Down
3 changes: 3 additions & 0 deletions daemon/routing.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,4 +180,7 @@ struct route_hop *get_route(tal_t *ctx, struct routing_state *rstate,
bool short_channel_id_from_str(const char *str, size_t strlen,
struct short_channel_id *dst);

bool short_channel_id_eq(const struct short_channel_id *a,
const struct short_channel_id *b);

#endif /* LIGHTNING_DAEMON_ROUTING_H */
10 changes: 5 additions & 5 deletions wire/fromwire.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,15 @@ void fromwire_channel_id(const u8 **cursor, size_t *max,
void fromwire_short_channel_id(const u8 **cursor, size_t *max,
struct short_channel_id *short_channel_id)
{
be32 txnum = 0;
u8 outnum;
be32 txnum = 0, blocknum = 0;

short_channel_id->blocknum = fromwire_u32(cursor, max);
/* Pulling 3 bytes off wire is tricky; they're big-endian. */
fromwire(cursor, max, (char *)&blocknum + 1, 3);
short_channel_id->blocknum = be32_to_cpu(blocknum);
fromwire(cursor, max, (char *)&txnum + 1, 3);
short_channel_id->txnum = be32_to_cpu(txnum);
fromwire(cursor, max, &outnum, 1);
short_channel_id->outnum = outnum;

short_channel_id->outnum = fromwire_u16 (cursor, max);
}

void fromwire_sha256(const u8 **cursor, size_t *max, struct sha256 *sha256)
Expand Down
7 changes: 7 additions & 0 deletions wire/peer_wire.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,10 @@ bool unknown_msg_discardable(const u8 *cursor)
enum wire_type t = fromwire_peektype(cursor);
return unknown_type(t) && (t & 1);
}

bool short_channel_id_eq(const struct short_channel_id *a,
const struct short_channel_id *b)
{
return a->blocknum == b->blocknum && a->txnum == b->txnum &&
a->outnum == b->outnum;
}
3 changes: 3 additions & 0 deletions wire/peer_wire.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@ bool unknown_msg_discardable(const u8 *cursor);
/* Return true if it's a gossip message. */
bool gossip_msg(u8 *cursor);

/* Compare two short_channel_ids and return true if they are the equal */
bool short_channel_id_eq(const struct short_channel_id *a,
const struct short_channel_id *b);
#endif /* LIGHTNING_WIRE_PEER_WIRE_H */
16 changes: 10 additions & 6 deletions wire/test/run-peer-wire.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ void fromwire_pad_orig(const u8 **cursor, size_t *max, size_t num);
#define fromwire_pad fromwire_pad_orig
#include "../towire.c"
#include "../fromwire.c"
#include "../peer_wire.c"
#undef towire_pad
#undef fromwire_pad

Expand Down Expand Up @@ -130,23 +131,23 @@ struct msg_revoke_and_ack {
};
struct msg_channel_update {
secp256k1_ecdsa_signature signature;
struct short_channel_id short_channel_id;
u32 timestamp;
u16 flags;
u16 expiry;
u32 htlc_minimum_msat;
u32 fee_base_msat;
u32 fee_proportional_millionths;
struct short_channel_id short_channel_id;
};
struct msg_funding_locked {
struct channel_id channel_id;
struct pubkey next_per_commitment_point;
};
struct msg_announcement_signatures {
struct channel_id channel_id;
struct short_channel_id short_channel_id;
secp256k1_ecdsa_signature announcement_node_signature;
secp256k1_ecdsa_signature announcement_bitcoin_signature;
struct short_channel_id short_channel_id;
};
struct msg_commitment_signed {
struct channel_id channel_id;
Expand Down Expand Up @@ -190,11 +191,11 @@ struct msg_channel_announcement {
secp256k1_ecdsa_signature node_signature_2;
secp256k1_ecdsa_signature bitcoin_signature_1;
secp256k1_ecdsa_signature bitcoin_signature_2;
struct short_channel_id short_channel_id;
struct pubkey node_id_1;
struct pubkey node_id_2;
struct pubkey bitcoin_key_1;
struct pubkey bitcoin_key_2;
struct short_channel_id short_channel_id;
u8 *features;
};
struct msg_init {
Expand Down Expand Up @@ -693,7 +694,8 @@ static struct msg_init *fromwire_struct_init(const tal_t *ctx, const void *p, si
static bool channel_announcement_eq(const struct msg_channel_announcement *a,
const struct msg_channel_announcement *b)
{
return eq_upto(a, b, features)
return eq_upto(a, b, short_channel_id) &&
short_channel_id_eq(&a->short_channel_id, &b->short_channel_id)
&& eq_var(a, b, features);
}

Expand All @@ -706,7 +708,8 @@ static bool funding_locked_eq(const struct msg_funding_locked *a,
static bool announcement_signatures_eq(const struct msg_announcement_signatures *a,
const struct msg_announcement_signatures *b)
{
return structeq(a, b);
return eq_upto(a, b, short_channel_id) &&
short_channel_id_eq(&a->short_channel_id, &b->short_channel_id);
}

static bool update_fail_htlc_eq(const struct msg_update_fail_htlc *a,
Expand Down Expand Up @@ -791,7 +794,8 @@ static bool open_channel_eq(const struct msg_open_channel *a,
static bool channel_update_eq(const struct msg_channel_update *a,
const struct msg_channel_update *b)
{
return structeq(a, b);
return eq_upto(a, b, short_channel_id) &&
short_channel_id_eq(&a->short_channel_id, &b->short_channel_id);
}

static bool accept_channel_eq(const struct msg_accept_channel *a,
Expand Down
6 changes: 3 additions & 3 deletions wire/towire.c
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ void towire_short_channel_id(u8 **pptr,
const struct short_channel_id *short_channel_id)
{
be32 txnum = cpu_to_be32(short_channel_id->txnum);
u8 outnum = short_channel_id->outnum;
be32 blocknum = cpu_to_be32(short_channel_id->blocknum);

towire_u32(pptr, short_channel_id->blocknum);
towire(pptr, (char *)&blocknum + 1, 3);
towire(pptr, (char *)&txnum + 1, 3);
towire(pptr, &outnum, 1);
towire_u16(pptr, short_channel_id->outnum);
}

void towire_sha256(u8 **pptr, const struct sha256 *sha256)
Expand Down
9 changes: 7 additions & 2 deletions wire/wire.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@
#include <ccan/short_types/short_types.h>
#include <stdlib.h>

/* Short Channel ID is composed of 3 bytes for the block height, 3
* bytes of tx index in block and 2 bytes of output index. The
* bitfield is mainly for unit tests where it is nice to be able to
* just memset them and not have to take care about the extra byte for
* u32 */
struct short_channel_id {
u32 blocknum;
u32 blocknum : 24;
u32 txnum : 24;
u8 outnum : 8;
u16 outnum;
};
struct channel_id {
u8 id[32];
Expand Down

0 comments on commit 05e951d

Please sign in to comment.