Skip to content

Commit

Permalink
cleanup: make 'u8 *features' and 'struct feature_set *fset' more expl…
Browse files Browse the repository at this point in the history
…icit.

It's almost always "their_features" and "our_features" respectively, so
make those names clear.

Suggested-by: @cdecker
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
  • Loading branch information
rustyrussell committed Apr 3, 2020
1 parent 28e3ffc commit 2f1502a
Show file tree
Hide file tree
Showing 31 changed files with 134 additions and 116 deletions.
6 changes: 3 additions & 3 deletions channeld/channel_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
# Begin! (passes gossipd-client fd)
msgtype,channel_init,1000
msgdata,channel_init,chainparams,chainparams,
msgdata,channel_init,feature_set,feature_set,
msgdata,channel_init,our_features,feature_set,
msgdata,channel_init,funding_txid,bitcoin_txid,
msgdata,channel_init,funding_txout,u16,
msgdata,channel_init,funding_satoshi,amount_sat,
Expand Down Expand Up @@ -64,8 +64,8 @@ msgdata,channel_init,init_peer_pkt_len,u16,
msgdata,channel_init,init_peer_pkt,u8,init_peer_pkt_len
msgdata,channel_init,reached_announce_depth,bool,
msgdata,channel_init,last_remote_secret,secret,
msgdata,channel_init,lflen,u16,
msgdata,channel_init,localfeatures,u8,lflen
msgdata,channel_init,flen,u16,
msgdata,channel_init,their_features,u8,flen
msgdata,channel_init,upfront_shutdown_script_len,u16,
msgdata,channel_init,upfront_shutdown_script,u8,upfront_shutdown_script_len
msgdata,channel_init,remote_ann_node_sig,?secp256k1_ecdsa_signature,
Expand Down
15 changes: 9 additions & 6 deletions channeld/channeld.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,10 @@ struct peer {
u64 next_index[NUM_SIDES];

/* Features peer supports. */
u8 *features;
u8 *their_features;

/* Features we support. */
struct feature_set *fset;
struct feature_set *our_features;

/* Tolerable amounts for feerate (only relevant for fundee). */
u32 feerate_min, feerate_max;
Expand Down Expand Up @@ -418,7 +418,9 @@ static void send_announcement_signatures(struct peer *peer)
static u8 *create_channel_announcement(const tal_t *ctx, struct peer *peer)
{
int first, second;
u8 *cannounce, *features = get_agreed_channelfeatures(tmpctx, peer->fset, peer->features);
u8 *cannounce, *features
= get_agreed_channelfeatures(tmpctx, peer->our_features,
peer->their_features);

if (peer->channel_direction == 0) {
first = LOCAL;
Expand Down Expand Up @@ -2328,7 +2330,8 @@ static void peer_reconnect(struct peer *peer,
bool dataloss_protect, check_extra_fields;
const u8 **premature_msgs = tal_arr(peer, const u8 *, 0);

dataloss_protect = feature_negotiated(peer->fset, peer->features,
dataloss_protect = feature_negotiated(peer->our_features,
peer->their_features,
OPT_DATA_LOSS_PROTECT);

/* Both these options give us extra fields to check. */
Expand Down Expand Up @@ -3059,7 +3062,7 @@ static void init_channel(struct peer *peer)
msg = wire_sync_read(tmpctx, MASTER_FD);
if (!fromwire_channel_init(peer, msg,
&chainparams,
&peer->fset,
&peer->our_features,
&funding_txid, &funding_txout,
&funding,
&minimum_depth,
Expand Down Expand Up @@ -3105,7 +3108,7 @@ static void init_channel(struct peer *peer)
&funding_signed,
&peer->announce_depth_reached,
&last_remote_per_commit_secret,
&peer->features,
&peer->their_features,
&peer->remote_upfront_shutdown_script,
&remote_ann_node_sig,
&remote_ann_bitcoin_sig,
Expand Down
14 changes: 8 additions & 6 deletions common/bolt11.c
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ static void shift_bitmap_down(u8 *bitmap, size_t bits)
* See [Feature Bits](#feature-bits).
*/
static char *decode_9(struct bolt11 *b11,
const struct feature_set *fset,
const struct feature_set *our_features,
struct hash_u5 *hu5,
u5 **data, size_t *data_len,
size_t data_length)
Expand All @@ -512,9 +512,10 @@ static char *decode_9(struct bolt11 *b11,
* - if the `9` field contains unknown _even_ bits that are non-zero:
* - MUST fail the payment.
*/
/* We skip this check for the cli tool, which sets fset to NULL */
if (fset) {
badf = features_unsupported(fset, b11->features, BOLT11_FEATURE);
/* We skip this check for the cli tool, which sets our_features to NULL */
if (our_features) {
badf = features_unsupported(our_features,
b11->features, BOLT11_FEATURE);
if (badf != -1)
return tal_fmt(b11, "9: unknown feature bit %i", badf);
}
Expand Down Expand Up @@ -545,7 +546,7 @@ struct bolt11 *new_bolt11(const tal_t *ctx,

/* Decodes and checks signature; returns NULL on error. */
struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
const struct feature_set *fset,
const struct feature_set *our_features,
const char *description, char **fail)
{
char *hrp, *amountstr, *prefix;
Expand Down Expand Up @@ -740,7 +741,8 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
data_length);
break;
case '9':
problem = decode_9(b11, fset, &hu5, &data, &data_len,
problem = decode_9(b11, our_features, &hu5,
&data, &data_len,
data_length);
break;
case 's':
Expand Down
2 changes: 1 addition & 1 deletion common/bolt11.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ struct bolt11 {
* fset is NULL to accept any features (usually not desirable!).
*/
struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
const struct feature_set *fset,
const struct feature_set *our_features,
const char *description, char **fail);

/* Initialize an empty bolt11 struct with optional amount */
Expand Down
33 changes: 17 additions & 16 deletions common/features.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,17 +158,17 @@ static void clear_feature_bit(u8 *features, u32 bit)
* it sets.
*/
u8 *get_agreed_channelfeatures(const tal_t *ctx,
const struct feature_set *ours,
const u8 *theirfeatures)
const struct feature_set *our_features,
const u8 *their_features)
{
u8 *f = tal_dup_talarr(ctx, u8, ours->bits[CHANNEL_FEATURE]);
u8 *f = tal_dup_talarr(ctx, u8, our_features->bits[CHANNEL_FEATURE]);
size_t max_len = 0;

/* Clear any features which they didn't offer too */
for (size_t i = 0; i < 8 * tal_count(f); i += 2) {
if (!feature_offered(f, i))
continue;
if (!feature_offered(theirfeatures, i)) {
if (!feature_offered(their_features, i)) {
clear_feature_bit(f, COMPULSORY_FEATURE(i));
clear_feature_bit(f, OPTIONAL_FEATURE(i));
continue;
Expand Down Expand Up @@ -197,11 +197,11 @@ bool feature_offered(const u8 *features, size_t f)
|| feature_is_set(features, OPTIONAL_FEATURE(f));
}

bool feature_negotiated(const struct feature_set *ours,
const u8 *lfeatures, size_t f)
bool feature_negotiated(const struct feature_set *our_features,
const u8 *their_features, size_t f)
{
return feature_offered(lfeatures, f)
&& feature_offered(ours->bits[INIT_FEATURE], f);
return feature_offered(their_features, f)
&& feature_offered(our_features->bits[INIT_FEATURE], f);
}

/**
Expand All @@ -215,7 +215,7 @@ bool feature_negotiated(const struct feature_set *ours,
*
* Returns -1 on success, or first unsupported feature.
*/
static int all_supported_features(const struct feature_set *ours,
static int all_supported_features(const struct feature_set *our_features,
const u8 *bitmap,
enum feature_place p)
{
Expand All @@ -226,24 +226,25 @@ static int all_supported_features(const struct feature_set *ours,
if (!test_bit(bitmap, bitnum/8, bitnum%8))
continue;

if (feature_offered(ours->bits[p], bitnum))
if (feature_offered(our_features->bits[p], bitnum))
continue;

return bitnum;
}
return -1;
}

int features_unsupported(const struct feature_set *ours, const u8 *theirs,
int features_unsupported(const struct feature_set *our_features,
const u8 *their_features,
enum feature_place p)
{
/* BIT 2 would logically be "compulsory initial_routing_sync", but
* that does not exist, so we special case it. */
if (feature_is_set(theirs,
if (feature_is_set(their_features,
COMPULSORY_FEATURE(OPT_INITIAL_ROUTING_SYNC)))
return COMPULSORY_FEATURE(OPT_INITIAL_ROUTING_SYNC);

return all_supported_features(ours, theirs, p);
return all_supported_features(our_features, their_features, p);
}

static const char *feature_name(const tal_t *ctx, size_t f)
Expand All @@ -269,12 +270,12 @@ static const char *feature_name(const tal_t *ctx, size_t f)
}

const char **list_supported_features(const tal_t *ctx,
const struct feature_set *ours)
const struct feature_set *fset)
{
const char **list = tal_arr(ctx, const char *, 0);

for (size_t i = 0; i < tal_bytelen(ours->bits[INIT_FEATURE]) * 8; i++) {
if (test_bit(ours->bits[INIT_FEATURE], i / 8, i % 8))
for (size_t i = 0; i < tal_bytelen(fset->bits[INIT_FEATURE]) * 8; i++) {
if (test_bit(fset->bits[INIT_FEATURE], i / 8, i % 8))
tal_arr_expand(&list, feature_name(list, i));
}

Expand Down
11 changes: 6 additions & 5 deletions common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,24 +32,25 @@ bool feature_set_or(struct feature_set *a,

/* Returns -1 if we're OK with all these offered features, otherwise first
* unsupported (even) feature. */
int features_unsupported(const struct feature_set *ours, const u8 *theirs,
int features_unsupported(const struct feature_set *our_features,
const u8 *their_features,
enum feature_place p);

/* For the features in channel_announcement */
u8 *get_agreed_channelfeatures(const tal_t *ctx,
const struct feature_set *ours,
const struct feature_set *our_features,
const u8 *theirfeatures);

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

/* Was this feature bit offered by them and us? */
bool feature_negotiated(const struct feature_set *ours,
const u8 *features, size_t f);
bool feature_negotiated(const struct feature_set *our_features,
const u8 *their_features, size_t f);

/* Return a list of what (init) features we advertize. */
const char **list_supported_features(const tal_t *ctx,
const struct feature_set *ours);
const struct feature_set *fset);

/* Low-level helpers to deal with big-endian bitfields. */
bool feature_is_set(const u8 *features, size_t bit);
Expand Down
2 changes: 1 addition & 1 deletion connectd/connect_wire.csv
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

msgtype,connectctl_init,2000
msgdata,connectctl_init,chainparams,chainparams,
msgdata,connectctl_init,feature_set,feature_set,
msgdata,connectctl_init,our_features,feature_set,
msgdata,connectctl_init,id,node_id,
msgdata,connectctl_init,num_wireaddrs,u16,
msgdata,connectctl_init,wireaddrs,wireaddr_internal,num_wireaddrs
Expand Down
Loading

0 comments on commit 2f1502a

Please sign in to comment.