rpc: Merge joinpsbts locktimes correctly #35100

nervana21 commented at 6:12 PM on April 17, 2026: contributor

joinpsbts previously took the minimum nLockTime across the PSBTs being merged, so the combined transaction could be minable earlier than the strictest party’s locktime. A merged PSBT should use the maximum nLockTime amongst the PSBTs that have at least one input with nSequence != SEQUENCE_FINAL.

PSBTs whose inputs are all SEQUENCE_FINAL do not constrain the locktime.

This PR implements the fix, updates RPC documentation, and extends rpc_psbt tests.

rpc: Merge joinpsbts locktimes correctly

A merged PSBT should use the maximum `nLockTime` amongst the PSBTs that
have at least one input with `nSequence` != `SEQUENCE_FINAL`. PSBTs
whose inputs are all `SEQUENCE_FINAL` do not constrain the locktime.

Document this in the help text and extend rpc_psbt tests.

0b00625fc8

DrahtBot added the label RPC/REST/ZMQ on Apr 17, 2026

DrahtBot commented at 6:13 PM on April 17, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#21283 (Implement BIP 370 PSBTv2 by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/rpc/rawtransaction.cpp:1835 in 0b00625fc8

1833 | +        const bool locktime_enabled = std::any_of(psbtx.tx->vin.begin(), psbtx.tx->vin.end(), [](const CTxIn& txin) {
1834 | +            return txin.nSequence != CTxIn::SEQUENCE_FINAL;
1835 | +        });
1836 | +        if (locktime_enabled && (!has_locktime_constraint || psbtx.tx->nLockTime > max_locktime)) {
1837 | +            max_locktime = psbtx.tx->nLockTime;
1838 | +            has_locktime_constraint = true;

Bicaru20 commented at 5:47 PM on April 20, 2026:

I don't understand why you need the has_locktime_constarint. If I am not wrong, remvoing it would not alter the logic of the if as it would still enter always that the current locktime is higher. I would suggest to eliminate it:

        // Choose the highest lock time
        if (locktime_enabled && psbtx.tx->nLockTime > max_locktime) {
            max_locktime = psbtx.tx->nLockTime;

in test/functional/rpc_psbt.py:932 in 0b00625fc8

 923 | @@ -923,6 +924,29 @@ def test_psbt_input_keys(psbt_input, keys):
 924 |          assert "final_scriptwitness" not in joined_decoded['inputs'][3]
 925 |          assert "final_scriptSig" not in joined_decoded['inputs'][3]
 926 |  
 927 | +        # Check that joinpsbts ignores locktime when all inputs use SEQUENCE_FINAL
 928 | +        addr5 = self.nodes[1].getnewaddress("", "bech32m")
 929 | +        utxo5 = self.create_outpoints(self.nodes[0], outputs=[{addr5: 5}])[0]
 930 | +        self.generate(self.nodes[0], 6)
 931 | +        vin = [{"txid": utxo4["txid"], "vout": utxo4["vout"], "sequence": SEQUENCE_FINAL}]
 932 | +        psbt_50 = self.nodes[1].createpsbt(vin, {self.nodes[0].getnewaddress():Decimal('4.999')}, locktime=50)

Bicaru20 commented at 6:08 PM on April 20, 2026:

Even if the sequence was valid, the psbt with locktime= 50 would be ignore as the maximum is 200. I think it is better to put a higher locktime than 200 to show the locktime is ignored

        psbt_250 = self.nodes[1].createpsbt(vin, {self.nodes[0].getnewaddress():Decimal('4.999')}, locktime=250)

Bicaru20 commented at 8:41 PM on April 20, 2026: none

Left some comment son the code.

Still, I think it should be discuss how to approach the case where we have a height-based timelock and time-based timelock. If I am not wrong, it is considered height-based if 0 < locktime < 500_000_000, and if it is higher than that it is time-based.

With the current approach, if we had a locktime of each type, the time-based would always be the max_locktime, whether or not the height-based is older. I am not sure if this should be the expected behavior, or if it is at least better than what we currently have, which selects the min_locktime.