Minor fixups on PR #3477

Feedback from @niftynei and me; nothing major, but avoids
another round-trip.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

common/features.c

@@ -257,19 +257,20 @@ const char **list_supported_features(const tal_t *ctx)
 u8 *featurebits_or(const tal_t *ctx, const u8 *f1 TAKES, const u8 *f2 TAKES)
 {
 	size_t l1 = tal_bytelen(f1), l2 = tal_bytelen(f2);
-	size_t lm = l1 > l2 ? l1 : l2;
-	u8 *result = tal_arrz(ctx, u8, lm);
+	u8 *result;
 
-	for (size_t i = 0; i < l1; i++)
-		result[lm - l1 + i] = f1[i];
+	/* Easier if f2 is shorter. */
+	if (l1 < l2)
+		return featurebits_or(ctx, f2, f1);
 
+	assert(l2 <= l1);
+	result = tal_dup_arr(ctx, u8, f1, l1, 0);
+
+	/* Note: features are packed to the end of the bitmap */
 	for (size_t i = 0; i < l2; i++)
-		result[lm - l2 + i] |= f2[i];
+		result[l1 - l2 + i] |= f2[i];
 
 	/* Cleanup the featurebits if we were told to do so. */
-	if (taken(f1))
-		tal_free(f1);
-
 	if (taken(f2))
 		tal_free(f2);
 

doc/PLUGINS.md

@@ -119,7 +119,7 @@ has been started. Critical plugins that should not be stopped should set it
 to false.
 
 The `features` object allows the plugin to register featurebits that should be
-announced in a number of places in the protocol. They can be used to signal
+announced in a number of places in [the protocol][bolt9]. They can be used to signal
 support for custom protocol extensions to direct peers, remote nodes and in
 invoices. Custom protocol extensions can be implemented for example using the
 `sendcustommsg` method and the `custommsg` hook, or the `sendonion` method and
@@ -878,3 +878,4 @@ compatibility should the semantics be changed in future.
 [sendcustommsg]: lightning-dev-sendcustommsg.7.html
 [oddok]: https://github.com/lightningnetwork/lightning-rfc/blob/master/00-introduction.md#its-ok-to-be-odd
 [spec]: [https://github.com/lightningnetwork/lightning-rfc]
+[bolt9]: https://github.com/lightningnetwork/lightning-rfc/blob/master/09-features.md

lightningd/invoice.c

@@ -1007,8 +1007,8 @@ static struct command_result *json_invoice(struct command *cmd,
 	 * This is a combination of natively supported features and featurebits
 	 * that plugins asked us to include in the invoice. */
 	info->b11->features = featurebits_or(
-	    info->b11, take(get_offered_bolt11features(info->b11)),
-	    take(plugins_collect_featurebits(tmpctx, cmd->ld->plugins,
+	    info->b11, take(get_offered_bolt11features(NULL)),
+	    take(plugins_collect_featurebits(NULL, cmd->ld->plugins,
 					     PLUGIN_FEATURES_INVOICE)));
 
 #if DEVELOPER

tests/test_plugin.py

@@ -4,7 +4,7 @@ from flaky import flaky  # noqa: F401
 from pyln.client import RpcError, Millisatoshi
 from pyln.proto import Invoice
 from utils import (
-    DEVELOPER, only_one, sync_blockheight, TIMEOUT, wait_for, TEST_NETWORK
+    DEVELOPER, only_one, sync_blockheight, TIMEOUT, wait_for, TEST_NETWORK, expected_features
 )
 
 import json
@@ -848,10 +848,8 @@ def test_plugin_feature_announce(node_factory):
 
     # Check the featurebits we've set in the `init` message from
     # feature-test.py. (1 << 101) results in 13 bytes featutebits (000d) and
-    # has a leading 0x20, the remainder is added to avoid false positives, but
-    # may cause failures if we add support for more native featurebits.
-    init = l1.daemon.is_in_log(r'\[OUT\] 001000.*000d2000000000000000000002aaa2')
-    assert(init is not None)  # Make sure we caught the log message
+    # has a leading 0x20.
+    assert l1.daemon.is_in_log(r'\[OUT\] 001000.*000d20{:0>24}'.format(expected_features()))
 
     # Check the invoice featurebit we set in feature-test.py
     inv = l1.rpc.invoice(123, 'lbl', 'desc')['bolt11']