psbt: fix PSBT mutation in the changeset

During the changeset calculation after the `openchannel2_sign`
hook.

So this commit patch the problem with the following change:

- Addressed an issue where `psbt_get_changeset` was modifying the original PSBT unnecessarily.
- This modification led to problems with a different hsmd, as referenced in [Issue #6672](#6672).
- Noted a potential optimization where only a subpart of the PSBT
needs to be cloned, as the mutation is specific to inputs.

Link: #6672
Suggested-by: Ken Sedgwick <[email protected]>
Co-Developed-by: Ken Sedgwick <[email protected]>
Signed-off-by: Vincenzo Palazzo <[email protected]>

bitcoin/psbt.c

@@ -36,7 +36,7 @@ struct wally_psbt *create_psbt(const tal_t *ctx, size_t num_inputs, size_t num_o
 	return psbt;
 }
 
-struct wally_psbt *clone_psbt(const tal_t *ctx, struct wally_psbt *psbt)
+struct wally_psbt *clone_psbt(const tal_t *ctx, const struct wally_psbt *psbt)
 {
 	struct wally_psbt *clone;
 	tal_wally_start();

bitcoin/psbt.h

@@ -48,7 +48,7 @@ struct wally_psbt *new_psbt(const tal_t *ctx,
  * @ctx - allocation context
  * @psbt - psbt to be cloned
  */
-struct wally_psbt *clone_psbt(const tal_t *ctx, struct wally_psbt *psbt);
+struct wally_psbt *clone_psbt(const tal_t *ctx, const struct wally_psbt *psbt);
 
 /**
  * psbt_is_finalized - Check if tx is ready to be extracted

bitcoin/test/run-tx-bitcoin_tx_2of2_input_witness_weight.c

@@ -45,7 +45,7 @@ struct amount_asset amount_sat_to_asset(struct amount_sat *sat UNNEEDED, const u
 struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED)
 { fprintf(stderr, "amount_tx_fee called!\n"); abort(); }
 /* Generated stub for clone_psbt */
-struct wally_psbt *clone_psbt(const tal_t *ctx UNNEEDED, struct wally_psbt *psbt UNNEEDED)
+struct wally_psbt *clone_psbt(const tal_t *ctx UNNEEDED, const struct wally_psbt *psbt UNNEEDED)
 { fprintf(stderr, "clone_psbt called!\n"); abort(); }
 /* Generated stub for fromwire */
 const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED)

common/psbt_open.c

@@ -60,7 +60,7 @@ static int compare_outputs_at(const struct output_set *a,
 static const u8 *linearize_input(const tal_t *ctx,
 				 const struct wally_psbt_input *in)
 {
-	struct wally_psbt *psbt = create_psbt(NULL, 1, 0, 0);
+	struct wally_psbt *psbt = create_psbt(tmpctx, 1, 0, 0);
 	size_t byte_len;
 
 	psbt->inputs[0] = *in;
@@ -153,7 +153,7 @@ static bool output_identical(const struct wally_psbt *a,
 static void sort_inputs(struct wally_psbt *psbt)
 {
 	/* Build an input map */
-	struct input_set *set = tal_arr(NULL,
+	struct input_set *set = tal_arr(tmpctx,
 					struct input_set,
 					psbt->num_inputs);
 
@@ -168,8 +168,6 @@ static void sort_inputs(struct wally_psbt *psbt)
 	for (size_t i = 0; i < tal_count(set); i++) {
 		psbt->inputs[i] = set[i].input;
 	}
-
-	tal_free(set);
 }
 
 static void sort_outputs(struct wally_psbt *psbt)
@@ -494,20 +492,26 @@ bool psbt_output_to_external(const struct wally_psbt_output *output)
 	return !(!result);
 }
 
-bool psbt_contribs_changed(struct wally_psbt *orig,
+bool psbt_contribs_changed(const struct wally_psbt *orig,
 			   struct wally_psbt *new)
 {
-	assert(orig->version == 2 && new->version == 2);
-
+	struct wally_psbt *orig_deep_copy;
 	struct psbt_changeset *cs;
 	bool ok;
-	cs = psbt_get_changeset(NULL, orig, new);
+
+	assert(orig->version == 2 && new->version == 2);
+	// The `psbt_get_changeset` function modifies the original PSBT, which
+	// is not needed by the .
+	//
+	// This behavior also caused issues with a different hsmd, as documented in:
+	// https://github.com/ElementsProject/lightning/issues/6672
+	orig_deep_copy = clone_psbt(tmpctx, orig);
+	cs = psbt_get_changeset(tmpctx, orig_deep_copy, new);
 
 	ok = tal_count(cs->added_ins) > 0 ||
 	    tal_count(cs->rm_ins) > 0 ||
 	    tal_count(cs->added_outs) > 0 ||
 	    tal_count(cs->rm_outs) > 0;
 
-	tal_free(cs);
 	return ok;
 }

common/psbt_open.h

@@ -190,9 +190,9 @@ bool psbt_output_to_external(const struct wally_psbt_output *output);
 /* psbt_contribs_changed - Returns true if the psbt's inputs/outputs
  *                         have changed.
  *
- * @orig - originating psbt
+ * @orig - originating psbt that will be untouch
  * @new  - 'updated' psbt, to verify is unchanged
  */
-bool psbt_contribs_changed(struct wally_psbt *orig,
+bool psbt_contribs_changed(const struct wally_psbt *orig,
 			   struct wally_psbt *new);
 #endif /* LIGHTNING_COMMON_PSBT_OPEN_H */