Skip to content

Commit

Permalink
features: more general accessor functions.
Browse files Browse the repository at this point in the history
As we add more features, the current code is insufficient.

1. Keep an array of single feature bits, for easy switching on and off.
2. Create feature_offered() which checks for both compulsory and optional
   variants.
3. Invert requires_unsupported_features() and unsupported_features()
   which tend to be double-negative, all_supported_features() and
   features_supported().
4. Move single feature definition from wire/peer_wire.h to common/features.h.

Signed-off-by: Rusty Russell <[email protected]>
  • Loading branch information
rustyrussell committed Mar 14, 2018
1 parent 1f443df commit 46cc7c2
Show file tree
Hide file tree
Showing 10 changed files with 145 additions and 82 deletions.
133 changes: 95 additions & 38 deletions common/features.c
Original file line number Diff line number Diff line change
@@ -1,62 +1,119 @@
#include "features.h"
#include <assert.h>
#include <ccan/array_size/array_size.h>
#include <wire/peer_wire.h>

static const u8 supported_local_features[]
= {LOCALFEATURES_INITIAL_ROUTING_SYNC};
static const u8 supported_global_features[]
= {};
static const u32 local_features[] = {
LOCAL_INITIAL_ROUTING_SYNC
};

u8 *get_supported_global_features(const tal_t *ctx)
static const u32 global_features[] = {
};

static void set_bit(u8 **ptr, u32 bit)
{
return tal_dup_arr(ctx, u8, supported_global_features,
sizeof(supported_global_features), 0);
if (bit / 8 >= tal_len(*ptr))
tal_resizez(ptr, (bit / 8) + 1);
(*ptr)[bit / 8] |= (1 << (bit % 8));
}

/* We don't insist on anything, it's all optional. */
static u8 *mkfeatures(const tal_t *ctx, const u32 *arr, size_t n)
{
u8 *f = tal_arr(ctx, u8, 0);

for (size_t i = 0; i < n; i++)
set_bit(&f, OPTIONAL_FEATURE(arr[i]));
return f;
}

u8 *get_supported_local_features(const tal_t *ctx)
u8 *get_offered_global_features(const tal_t *ctx)
{
return tal_dup_arr(ctx, u8, supported_local_features,
sizeof(supported_local_features), 0);
return mkfeatures(ctx, global_features, ARRAY_SIZE(global_features));
}

u8 *get_offered_local_features(const tal_t *ctx)
{
return mkfeatures(ctx, local_features, ARRAY_SIZE(local_features));
}

static bool test_bit(const u8 *features, size_t byte, unsigned int bit)
{
return features[byte] & (1 << (bit % 8));
}

static bool feature_set(const u8 *features, size_t bit)
{
size_t bytenum = bit / 8;

if (bytenum >= tal_len(features))
return false;

return test_bit(features, bytenum, bit % 8);
}

bool feature_offered(const u8 *features, size_t f)
{
assert(f % 2 == 0);

return feature_set(features, f)
|| feature_set(features, OPTIONAL_FEATURE(f));
}

static bool feature_supported(int feature_bit,
const u32 *supported,
size_t num_supported)
{
for (size_t i = 0; i < num_supported; i++) {
if (supported[i] == feature_bit)
return true;
}
return false;
}

/**
* requires_unsupported_features - Check if we support what's being asked
* all_supported_features - Check if we support what's being asked
*
* Given the features vector that the remote connection is expecting
* from us, we check to see if we support all even bit features, i.e.,
* the required features. We do so by subtracting our own features in
* the provided positions and see if even bits remain.
* the required features.
*
* @bitmap: the features bitmap the peer is asking for
* @supportmap: what do we support
* @smlen: how long is our supportmap
* @supported: array of features we support
* @num_supported: how many elements in supported
*/
static bool requires_unsupported_features(const u8 *bitmap,
const u8 *supportmap,
size_t smlen)
static bool all_supported_features(const u8 *bitmap,
const u32 *supported,
size_t num_supported)
{
size_t len = tal_count(bitmap);
u8 support;
for (size_t i=0; i<len; i++) {
/* Find matching bitmap byte in supportmap, 0x00 if none */
if (len > smlen) {
support = 0x00;
} else {
support = supportmap[smlen-1];
}

/* Cancel out supported bits, check for even bits */
if ((~support & bitmap[i]) & 0x55)
return true;

/* It's OK to be odd: only check even bits. */
for (size_t bitnum = 0; bitnum < len; bitnum += 2) {
if (!test_bit(bitmap, bitnum/8, bitnum%8))
continue;

if (feature_supported(bitnum, supported, num_supported))
continue;

return false;
}
return false;
return true;
}

bool unsupported_features(const u8 *gfeatures, const u8 *lfeatures)
bool features_supported(const u8 *gfeatures, const u8 *lfeatures)
{
return requires_unsupported_features(gfeatures,
supported_global_features,
sizeof(supported_global_features))
|| requires_unsupported_features(lfeatures,
supported_local_features,
sizeof(supported_local_features));
/* BIT 2 would logically be "compulsory initial_routing_sync", but
* that does not exist, so we special case it. */
if (feature_set(lfeatures,
COMPULSORY_FEATURE(LOCAL_INITIAL_ROUTING_SYNC)))
return false;

return all_supported_features(gfeatures,
global_features,
ARRAY_SIZE(global_features))
|| all_supported_features(lfeatures,
local_features,
ARRAY_SIZE(local_features));
}

27 changes: 23 additions & 4 deletions common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,30 @@
#include <ccan/short_types/short_types.h>
#include <ccan/tal/tal.h>

/* Returns true if these contain any unsupported features. */
bool unsupported_features(const u8 *gfeatures, const u8 *lfeatures);
/* Returns true if we're OK with all these offered features. */
bool features_supported(const u8 *gfeatures, const u8 *lfeatures);

/* For sending our features: tal_len() returns length. */
u8 *get_supported_global_features(const tal_t *ctx);
u8 *get_supported_local_features(const tal_t *ctx);
u8 *get_offered_global_features(const tal_t *ctx);
u8 *get_offered_local_features(const tal_t *ctx);

/* Is this feature bit requested? (Either compulsory or optional) */
bool feature_offered(const u8 *features, size_t f);

#define COMPULSORY_FEATURE(x) (x)
#define OPTIONAL_FEATURE(x) ((x)+1)

/* BOLT #9:
*
* ## Assigned `localfeatures` flags
*...
* | Bits | Name |...
* | 0/1 | `option-data-loss-protect` |...
* | 3 | `initial_routing_sync` |...
* | 4/5 | `option_upfront_shutdown_script` |...
*/
#define LOCAL_DATA_LOSS_PROTECT 0
#define LOCAL_INITIAL_ROUTING_SYNC 2
#define LOCAL_UPFRONT_SHUTDOWN_SCRIPT 4

#endif /* LIGHTNING_COMMON_FEATURES_H */
4 changes: 2 additions & 2 deletions gossipd/routing.c
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ u8 *handle_channel_announcement(struct routing_state *rstate,
* and MUST NOT add the channel to its local network view, and
* SHOULD NOT forward the announcement.
*/
if (unsupported_features(features, NULL)) {
if (!features_supported(features, NULL)) {
status_trace("Ignoring channel announcement, unsupported features %s.",
tal_hex(pending, features));
goto ignored;
Expand Down Expand Up @@ -1070,7 +1070,7 @@ u8 *handle_node_announcement(struct routing_state *rstate, const u8 *node_ann)
* receiving node MUST NOT parse the remainder of the message
* and MAY discard the message altogether.
*/
if (unsupported_features(features, NULL)) {
if (!features_supported(features, NULL)) {
status_trace("Ignoring node announcement for node %s, unsupported features %s.",
type_to_string(tmpctx, struct pubkey, &node_id),
tal_hex(tmpctx, features));
Expand Down
2 changes: 1 addition & 1 deletion gossipd/test/run-bench-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const char *onion_type_name(int e UNNEEDED)
bool replace_broadcast(const tal_t *ctx UNNEEDED,
struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
const u8 *payload UNNEEDED)
const u8 *payload TAKES UNNEEDED)
{ fprintf(stderr, "replace_broadcast called!\n"); abort(); }
/* Generated stub for sanitize_error */
char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
Expand Down
2 changes: 1 addition & 1 deletion gossipd/test/run-find_route-specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ const char *onion_type_name(int e UNNEEDED)
bool replace_broadcast(const tal_t *ctx UNNEEDED,
struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
const u8 *payload UNNEEDED)
const u8 *payload TAKES UNNEEDED)
{ fprintf(stderr, "replace_broadcast called!\n"); abort(); }
/* Generated stub for sanitize_error */
char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
Expand Down
2 changes: 1 addition & 1 deletion gossipd/test/run-find_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const char *onion_type_name(int e UNNEEDED)
bool replace_broadcast(const tal_t *ctx UNNEEDED,
struct broadcast_state *bstate UNNEEDED,
u64 *index UNNEEDED,
const u8 *payload UNNEEDED)
const u8 *payload TAKES UNNEEDED)
{ fprintf(stderr, "replace_broadcast called!\n"); abort(); }
/* Generated stub for sanitize_error */
char *sanitize_error(const tal_t *ctx UNNEEDED, const u8 *errmsg UNNEEDED,
Expand Down
6 changes: 3 additions & 3 deletions lightningd/gossip_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ static void peer_nongossip(struct subd *gossip, const u8 *msg,
tal_hex(msg, msg));

/* We already checked the features when it first connected. */
if (unsupported_features(gfeatures, lfeatures)) {
if (!features_supported(gfeatures, lfeatures)) {
log_unusual(gossip->log,
"Gossip gave unsupported features %s/%s",
tal_hex(msg, gfeatures),
Expand Down Expand Up @@ -204,8 +204,8 @@ void gossip_init(struct lightningd *ld)
msg = towire_gossipctl_init(
tmpctx, ld->config.broadcast_interval,
&get_chainparams(ld)->genesis_blockhash, &ld->id, ld->portnum,
get_supported_global_features(tmpctx),
get_supported_local_features(tmpctx), ld->wireaddrs, ld->rgb,
get_offered_global_features(tmpctx),
get_offered_local_features(tmpctx), ld->wireaddrs, ld->rgb,
ld->alias, ld->config.channel_update_interval);
subd_send_msg(ld->gossip, msg);
tal_free(tmpctx);
Expand Down
20 changes: 10 additions & 10 deletions lightningd/peer_control.c
Original file line number Diff line number Diff line change
Expand Up @@ -282,8 +282,8 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
struct crypto_state cs;
u8 *gfeatures, *lfeatures;
u8 *error;
u8 *supported_global_features;
u8 *supported_local_features;
u8 *global_features;
u8 *local_features;
struct channel *channel;
struct wireaddr addr;
u64 gossip_index;
Expand All @@ -295,22 +295,22 @@ void peer_connected(struct lightningd *ld, const u8 *msg,
fatal("Gossip gave bad GOSSIP_PEER_CONNECTED message %s",
tal_hex(msg, msg));

if (unsupported_features(gfeatures, lfeatures)) {
if (!features_supported(gfeatures, lfeatures)) {
log_unusual(ld->log, "peer %s offers unsupported features %s/%s",
type_to_string(msg, struct pubkey, &id),
tal_hex(msg, gfeatures),
tal_hex(msg, lfeatures));
supported_global_features = get_supported_global_features(msg);
supported_local_features = get_supported_local_features(msg);
global_features = get_offered_global_features(msg);
local_features = get_offered_local_features(msg);
error = towire_errorfmt(msg, NULL,
"We only support globalfeatures %s"
"We only offer globalfeatures %s"
" and localfeatures %s",
tal_hexstr(msg,
supported_global_features,
tal_len(supported_global_features)),
global_features,
tal_len(global_features)),
tal_hexstr(msg,
supported_local_features,
tal_len(supported_local_features)));
local_features,
tal_len(local_features)));
goto send_error;
}

Expand Down
18 changes: 9 additions & 9 deletions wallet/test/run-wallet.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ bool derive_basepoints(const struct privkey *seed UNNEEDED,
/* Generated stub for extract_channel_id */
bool extract_channel_id(const u8 *in_pkt UNNEEDED, struct channel_id *channel_id UNNEEDED)
{ fprintf(stderr, "extract_channel_id called!\n"); abort(); }
/* Generated stub for features_supported */
bool features_supported(const u8 *gfeatures UNNEEDED, const u8 *lfeatures UNNEEDED)
{ fprintf(stderr, "features_supported called!\n"); abort(); }
/* Generated stub for fromwire_gossipctl_peer_disconnect_reply */
bool fromwire_gossipctl_peer_disconnect_reply(const void *p UNNEEDED)
{ fprintf(stderr, "fromwire_gossipctl_peer_disconnect_reply called!\n"); abort(); }
Expand All @@ -90,12 +93,12 @@ enum watch_result funding_spent(struct channel *channel UNNEEDED,
/* Generated stub for get_feerate */
u32 get_feerate(const struct chain_topology *topo UNNEEDED, enum feerate feerate UNNEEDED)
{ fprintf(stderr, "get_feerate called!\n"); abort(); }
/* Generated stub for get_supported_global_features */
u8 *get_supported_global_features(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "get_supported_global_features called!\n"); abort(); }
/* Generated stub for get_supported_local_features */
u8 *get_supported_local_features(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "get_supported_local_features called!\n"); abort(); }
/* Generated stub for get_offered_global_features */
u8 *get_offered_global_features(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "get_offered_global_features called!\n"); abort(); }
/* Generated stub for get_offered_local_features */
u8 *get_offered_local_features(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "get_offered_local_features called!\n"); abort(); }
/* Generated stub for invoices_create */
bool invoices_create(struct invoices *invoices UNNEEDED,
struct invoice *pinvoice UNNEEDED,
Expand Down Expand Up @@ -356,9 +359,6 @@ u8 *towire_gossip_disable_channel(const tal_t *ctx UNNEEDED, const struct short_
/* Generated stub for towire_gossip_getpeers_request */
u8 *towire_gossip_getpeers_request(const tal_t *ctx UNNEEDED, const struct pubkey *id UNNEEDED)
{ fprintf(stderr, "towire_gossip_getpeers_request called!\n"); abort(); }
/* Generated stub for unsupported_features */
bool unsupported_features(const u8 *gfeatures UNNEEDED, const u8 *lfeatures UNNEEDED)
{ fprintf(stderr, "unsupported_features called!\n"); abort(); }
/* Generated stub for watch_txid */
struct txwatch *watch_txid(const tal_t *ctx UNNEEDED,
struct chain_topology *topo UNNEEDED,
Expand Down
13 changes: 0 additions & 13 deletions wire/peer_wire.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,6 @@ bool extract_channel_id(const u8 *in_pkt, struct channel_id *channel_id);
*/
#define CHANNEL_FLAGS_ANNOUNCE_CHANNEL 1

/* BOLT #7:
*
* Upon establishing a connection, the two endpoints negotiate whether
* to perform an initial sync by setting the `initial_routing_sync`
* flags in the `init` message. The endpoint SHOULD set the
* `initial_routing_sync` flag if it requires a full copy of the other
* endpoint's routing state. Upon receiving an `init` message with the
* `initial_routing_sync` flag set the node sends `channel_announcement`s,
* `channel_update`s and `node_announcement`s for all known channels and
* nodes as if they were just received.
*/
#define LOCALFEATURES_INITIAL_ROUTING_SYNC 0x08

/* BOLT #2:
*
* The sender MUST set `funding_satoshis` to less than 2^24 satoshi.
Expand Down

0 comments on commit 46cc7c2

Please sign in to comment.