From 4359f46ffde5424b277f69cd7738b367ff999a6e Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 15 May 2018 15:32:21 +0930 Subject: [PATCH] features: fix features_supported() function. 1. We need to test all bits, not all bytes. 2. Both local and global features need to be supported. 3. Untested code is broken code. Signed-off-by: Rusty Russell --- common/features.c | 4 +- common/test/run-features.c | 106 +++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 common/test/run-features.c diff --git a/common/features.c b/common/features.c index 961cc80e3116..81b42eabc138 100644 --- a/common/features.c +++ b/common/features.c @@ -98,7 +98,7 @@ static bool all_supported_features(const u8 *bitmap, const u32 *supported, size_t num_supported) { - size_t len = tal_count(bitmap); + size_t len = tal_count(bitmap) * 8; /* It's OK to be odd: only check even bits. */ for (size_t bitnum = 0; bitnum < len; bitnum += 2) { @@ -124,7 +124,7 @@ bool features_supported(const u8 *gfeatures, const u8 *lfeatures) return all_supported_features(gfeatures, global_features, ARRAY_SIZE(global_features)) - || all_supported_features(lfeatures, + && all_supported_features(lfeatures, local_features, ARRAY_SIZE(local_features)); } diff --git a/common/test/run-features.c b/common/test/run-features.c new file mode 100644 index 000000000000..9e201f5ba4bf --- /dev/null +++ b/common/test/run-features.c @@ -0,0 +1,106 @@ +#include "../features.c" +#include +#include +#include +#include +#include +#include + +/* AUTOGENERATED MOCKS START */ +/* AUTOGENERATED MOCKS END */ + +int main(void) +{ + u8 *bits, *lf, *gf; + + setup_locale(); + secp256k1_ctx = wally_get_secp_context(); + setup_tmpctx(); + + bits = tal_arr(tmpctx, u8, 0); + for (size_t i = 0; i < 100; i += 3) + set_bit(&bits, i); + for (size_t i = 0; i < 100; i++) + assert(test_bit(bits, i / 8, i % 8) == ((i % 3) == 0)); + + for (size_t i = 0; i < 100; i++) + assert(feature_set(bits, i) == ((i % 3) == 0)); + + /* Simple test: single byte */ + bits = tal_arr(tmpctx, u8, 1); + + /* Compulsory feature */ + bits[0] = (1 << 0); + assert(feature_offered(bits, 0)); + assert(!feature_offered(bits, 2)); + assert(!feature_offered(bits, 8)); + assert(!feature_offered(bits, 16)); + + /* Optional feature */ + bits[0] = (1 << 1); + assert(feature_offered(bits, 0)); + assert(!feature_offered(bits, 2)); + assert(!feature_offered(bits, 8)); + assert(!feature_offered(bits, 16)); + + /* Endian-sensitive test: big-endian means we frob last byte here */ + bits = tal_arrz(tmpctx, u8, 2); + + bits[1] = (1 << 0); + assert(feature_offered(bits, 0)); + assert(!feature_offered(bits, 2)); + assert(!feature_offered(bits, 8)); + assert(!feature_offered(bits, 16)); + + /* Optional feature */ + bits[1] = (1 << 1); + assert(feature_offered(bits, 0)); + assert(!feature_offered(bits, 2)); + assert(!feature_offered(bits, 8)); + assert(!feature_offered(bits, 16)); + + /* We always support no features. */ + memset(bits, 0, tal_len(bits)); + assert(features_supported(bits, bits)); + + /* We must support our own features. */ + lf = get_offered_global_features(tmpctx); + gf = get_offered_global_features(tmpctx); + assert(features_supported(gf, lf)); + + /* We can add random odd features, no problem. */ + for (size_t i = 1; i < 16; i += 2) { + bits = tal_dup_arr(tmpctx, u8, lf, tal_len(lf), 0); + set_bit(&bits, i); + assert(features_supported(gf, bits)); + + bits = tal_dup_arr(tmpctx, u8, gf, tal_len(gf), 0); + set_bit(&bits, i); + assert(features_supported(bits, lf)); + } + + /* We can't add random even features. */ + for (size_t i = 0; i < 16; i += 2) { + bits = tal_dup_arr(tmpctx, u8, lf, tal_len(lf), 0); + set_bit(&bits, i); + + /* Special case for missing compulsory feature */ + if (i == 2) { + assert(!features_supported(gf, bits)); + } else { + assert(features_supported(gf, bits) + == feature_supported(i, local_features, + ARRAY_SIZE(local_features))); + } + + bits = tal_dup_arr(tmpctx, u8, gf, tal_len(gf), 0); + set_bit(&bits, i); + assert(features_supported(bits, lf) + == feature_supported(i, global_features, + ARRAY_SIZE(global_features))); + } + + wally_cleanup(0); + tal_free(tmpctx); + return 0; +}