sign_psbt: use shared signing tx validation code
What the developer wrote
Authored by Jon Griffiths
sign_psbt: use shared signing tx validation code
This means we don't have to find the fee ourselves, and ensures that
the checks we perform (e.g. duplicate fees) are consistent whether we
are signing a tx or psbt.
What changed, and why it matters
This commit refactors how the Blockstream Jade hardware wallet checks transaction fees when signing a PSBT. Previously, the fee was calculated and validated inside the PSBT-specific code. Now it uses a shared validation routine that is also used for regular transaction signing. The goal is consistency: the same duplicate-fee and fee-validation checks apply regardless of whether the user is signing a raw transaction or a PSBT. The commit removes several local fee checks (such as rejecting blinded fee outputs or unexpected fee asset IDs) and relies on the shared function instead. This is a code-quality and consistency improvement, not a clear-cut security fix, but it touches security-critical fee logic.
Review the implementation of params_txn_validate() to confirm it enforces every constraint removed from validate_outputs(), especially for Liquid: fee outputs must be unblinded, must use the policy asset, and must not be duplicated. Ensure JADE_ASSERT-only replacements are acceptable for production builds. Run regression tests on Liquid PSBTs with blinded outputs, wrong asset IDs, and duplicate fee outputs.
Security signals we found
Refactoring of security-critical fee validation logic
Removal of explicit runtime checks for Liquid fee outputs (blinding, asset ID, fee tallying)
Replacement of runtime checks with debug-only JADE_ASSERT assertions
Introduction of shared validation routine params_txn_validate()
Potential for behavioral change if shared routine does not enforce all previously checked constraints
Evidence from the diff
In main/process/sign_psbt.c, validate_outputs() no longer computes explicit_fee or performs Liquid-specific fee-output checks (blinding status, asset-id match, fee tallying). Instead, sign_psbt() now extracts the transaction from the PSBT and calls params_txn_validate() before proceeding. That shared helper computes explicit_fee and presumably performs equivalent validation. The local checks are replaced by JADE_ASSERT() statements, which only fire in debug builds. The change reduces duplicated logic and aims to keep PSBT and raw-tx validation in sync. Because the diff does not show the implementation of params_txn_validate(), we cannot verify whether all removed checks are preserved.
Changed components
main/process/sign_psbt.cPSBT signing flowLiquid fee-output validationTransaction fee validationInspect captured patch +15 / −25
diff --git a/main/process/sign_psbt.c b/main/process/sign_psbt.c
index e3d44ac..d90a52d 100644
--- a/main/process/sign_psbt.c
+++ b/main/process/sign_psbt.c
@@ -450,24 +450,18 @@ static bool get_suitable_descriptor_record(const key_iter* iter, const uint32_t*
// Examine outputs for change we can automatically validate
static bool validate_outputs(const network_t network_id, struct wally_psbt* psbt, const uint8_t signing_flags,
const char* wallet_name, const multisig_data_t* multisig_data, const descriptor_data_t* descriptor,
- output_info_t* output_info, uint64_t* explicit_fee, const char** errmsg)
+ output_info_t* output_info, const char** errmsg)
{
JADE_ASSERT(network_id != NETWORK_NONE);
JADE_ASSERT(psbt);
// wallet_name, multisig_data and descriptor optional
JADE_ASSERT(output_info);
- JADE_INIT_OUT_SIZE(explicit_fee);
JADE_INIT_OUT_PPTR(errmsg);
const bool is_liquid = network_is_liquid(network_id);
JADE_ASSERT(!multisig_data || !descriptor); // cannot have both
JADE_ASSERT(!is_liquid || !descriptor); // atm do not support liquid descriptors
- uint8_t policy_asset[ASSET_TAG_LEN];
- if (is_liquid) {
- network_to_policy_asset(network_id, policy_asset, sizeof(policy_asset));
- }
-
key_iter iter; // Holds any public key in use
// Check each output in turn
@@ -500,20 +494,10 @@ static bool validate_outputs(const network_t network_id, struct wally_psbt* psbt
}
if (wally_psbt_get_output_script_len(psbt, index, &written) != WALLY_OK || !written) {
- if (outinfo->flags & OUTPUT_FLAG_CONFIDENTIAL || !(outinfo->flags & OUTPUT_FLAG_HAS_UNBLINDED)) {
- *errmsg = "Fee output (without script) cannot be blinded";
- return false;
- }
-
- if (memcmp(outinfo->asset_id, policy_asset, sizeof(policy_asset))) {
- *errmsg = "Unexpected fee output (without script) asset-id";
- return false;
- }
-
- // Tally fees
- *explicit_fee += outinfo->value;
-
- // If is fee output, can't be change, so may as well skip now
+ // Fee output
+ JADE_ASSERT(!(outinfo->flags & OUTPUT_FLAG_CONFIDENTIAL));
+ JADE_ASSERT(outinfo->flags & OUTPUT_FLAG_HAS_UNBLINDED);
+ // Fee outputs can't be change, so may as well skip now
JADE_ASSERT(!(outinfo->flags & (OUTPUT_FLAG_VALIDATED | OUTPUT_FLAG_CHANGE)));
continue;
}
@@ -674,6 +658,7 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
JADE_ASSERT(psbt);
JADE_INIT_OUT_PPTR(errmsg);
JADE_ASSERT(network_id != NETWORK_NONE);
+ int retval = 0;
size_t is_elements = 0;
JADE_WALLY_VERIFY(wally_psbt_is_elements(psbt, &is_elements));
@@ -682,7 +667,9 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
return CBOR_RPC_BAD_PARAMETERS;
}
+ uint64_t explicit_fee = 0;
struct wally_tx* tx = NULL; // Holds the extracted tx
+
// Fetch the tx to sign
if (psbt->version == WALLY_PSBT_VERSION_0) {
tx = psbt->tx; // For v0, use the PSBT tx directly
@@ -692,9 +679,13 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
}
JADE_ASSERT(tx && tx->num_inputs == psbt->num_inputs && tx->num_outputs == psbt->num_outputs);
+ if (!params_txn_validate(network_id, for_liquid, tx, &explicit_fee, errmsg)) {
+ retval = CBOR_RPC_BAD_PARAMETERS;
+ goto cleanup;
+ }
+
key_iter iter; // Holds any public/private key in use
SENSITIVE_PUSH(&iter, sizeof(iter));
- int retval = 0;
// We track if the type of the inputs we are signing changes (ie. single-sig vs
// green/multisig/other) so we can show a warning to the user if so.
@@ -842,9 +833,8 @@ int sign_psbt(const network_t network_id, struct wally_psbt* psbt, const char**
} // iterate keys
// Examine outputs for liquid unblinded info and fees, and for change we can automatically validate
- uint64_t explicit_fee = 0;
- if (!validate_outputs(network_id, psbt, signing_flags, wallet_name, multisig_data, descriptor, output_info,
- &explicit_fee, errmsg)) {
+ if (!validate_outputs(
+ network_id, psbt, signing_flags, wallet_name, multisig_data, descriptor, output_info, errmsg)) {
// errmsg will be populated
retval = CBOR_RPC_BAD_PARAMETERS;
goto cleanup;
Why this scored 44/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.