Skip to content

txprepare does not use provided feerate argument correctly #8164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ca-ruz opened this issue Mar 14, 2025 · 3 comments · May be fixed by #8234 or #8236
Open

txprepare does not use provided feerate argument correctly #8164

ca-ruz opened this issue Mar 14, 2025 · 3 comments · May be fixed by #8234 or #8236
Assignees
Labels
Milestone

Comments

@ca-ruz
Copy link

ca-ruz commented Mar 14, 2025

The txprepare function in Core Lightning does not seem to respect the feerate argument provided by the user. Instead of constructing the transaction with the specified feerate, it appears to apply some internal logic that results in a different feerate being used.

Additionally, the documentation for txprepare lacks clarity regarding the feerate parameter. It does not specify:

-The expected format or units for the feerate argument
-Whether alternative feerate options (besides "normal") are supported
-How txprepare processes the provided feerate and why the resulting transaction feerate differs from the input

Steps to Reproduce:

  1. Call txprepare with a specific feerate, e.g.:
lightning-cli txprepare ["outputs"] ["feerate"]
  1. Inspect the resulting transaction's actual feerate.
  2. Compare the expected feerate with the actual feerate applied by Core Lightning.

Expected Behavior:
The transaction should be created with the feerate explicitly passed to txprepare.

Actual Behavior:
The resulting transaction has a different feerate from the one provided.

Additional Information:

-Core Lightning version: v25.02
-Logs and output from txprepare:

(bumpchannelopen) orangepi@orangepizero3:~/code/bumpchannelopen$ l1-cli txprepare -k outputs="[{\"$ADDR\": 1000}]" feerate=4000
{
   "unsigned_tx": "0200000001aaf54e3195558eebfea8d3d2be25620dfc69ac8339a5c8b9982e97563ae94fc60100000000fdffffff02e80300000000000016001444e7df72c8aeac246153dd551b89e2860ce19eded197e605000000002251205365180cca6f315eea8a3b59e6532858298756b6c2549ca967a21b759317e70aed010000",
   "txid": "3a50bcd44ef2497eac2cbbe906b7fceb1747f5c189eb46e64f4357bfa349c769",
   "psbt": "cHNidP8BAgQCAAAAAQME7QEAAAEEAQEBBQECAQYBAwH7BAIAAAAAAQDdAgAAAAKXEKSq0E0DQaPS2uWtGeR3lAnhO6Ae5KrL3a+N1bp7xQAAAAAA/f///1HBXf9f/DfZnrfY9mZbqm6W3ILPb5J6wFUWtksSqxfvAQAAAAD9////A+DIEAAAAAAAIgAg081ivd8zkSVXb8P45RZEMbq54MTdIGT4ZtGPBUnJwFsanuYFAAAAACJRIGhKYhWSrVNDriJOXkCfku5ta+ehjeO/Sq96cLdZGkK28Fn0BQAAAAAiUSA3tWVCL3cQ8/9G4878aGFsfIfD0lQW2IJRxf7UPVi0PuwBAAABASsanuYFAAAAACJRIGhKYhWSrVNDriJOXkCfku5ta+ehjeO/Sq96cLdZGkK2AQ4gqvVOMZVVjuv+qNPSviViDfxprIM5pci5mC6XVjrpT8YBDwQBAAAAARAE/f///wABAwjoAwAAAAAAAAEEFgAUROffcsiurCRhU91VG4nihgzhnt4AAQMI0ZfmBQAAAAABBCJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cKAA=="
}
(bumpchannelopen) orangepi@orangepizero3:~/code/bumpchannelopen$ l1-cli setpsbtversion -k psbt="cHNidP8BAgQCAAAAAQME7QEAAAEEAQEBBQECAQYBAwH7BAIAAAAAAQDdAgAAAAKXEKSq0E0DQaPS2uWtGeR3lAnhO6Ae5KrL3a+N1bp7xQAAAAAA/f///1HBXf9f/DfZnrfY9mZbqm6W3ILPb5J6wFUWtksSqxfvAQAAAAD9////A+DIEAAAAAAAIgAg081ivd8zkSVXb8P45RZEMbq54MTdIGT4ZtGPBUnJwFsanuYFAAAAACJRIGhKYhWSrVNDriJOXkCfku5ta+ehjeO/Sq96cLdZGkK28Fn0BQAAAAAiUSA3tWVCL3cQ8/9G4878aGFsfIfD0lQW2IJRxf7UPVi0PuwBAAABASsanuYFAAAAACJRIGhKYhWSrVNDriJOXkCfku5ta+ehjeO/Sq96cLdZGkK2AQ4gqvVOMZVVjuv+qNPSviViDfxprIM5pci5mC6XVjrpT8YBDwQBAAAAARAE/f///wABAwjoAwAAAAAAAAEEFgAUROffcsiurCRhU91VG4nihgzhnt4AAQMI0ZfmBQAAAAABBCJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cKAA==" version=0
{
   "psbt": "cHNidP8BAH0CAAAAAar1TjGVVY7r/qjT0r4lYg38aayDOaXIuZgul1Y66U/GAQAAAAD9////AugDAAAAAAAAFgAUROffcsiurCRhU91VG4nihgzhnt7Rl+YFAAAAACJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cK7QEAAAABAN0CAAAAApcQpKrQTQNBo9La5a0Z5HeUCeE7oB7kqsvdr43VunvFAAAAAAD9////UcFd/1/8N9met9j2Zluqbpbcgs9vknrAVRa2SxKrF+8BAAAAAP3///8D4MgQAAAAAAAiACDTzWK93zORJVdvw/jlFkQxurngxN0gZPhm0Y8FScnAWxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrbwWfQFAAAAACJRIDe1ZUIvdxDz/0bjzvxoYWx8h8PSVBbYglHF/tQ9WLQ+7AEAAAEBKxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrYAAAA="
}
(bumpchannelopen) orangepi@orangepizero3:~/code/bumpchannelopen$ bt-cli analyzepsbt "cHNidP8BAH0CAAAAAar1TjGVVY7r/qjT0r4lYg38aayDOaXIuZgul1Y66U/GAQAAAAD9////AugDAAAAAAAAFgAUROffcsiurCRhU91VG4nihgzhnt7Rl+YFAAAAACJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cK7QEAAAABAN0CAAAAApcQpKrQTQNBo9La5a0Z5HeUCeE7oB7kqsvdr43VunvFAAAAAAD9////UcFd/1/8N9met9j2Zluqbpbcgs9vknrAVRa2SxKrF+8BAAAAAP3///8D4MgQAAAAAAAiACDTzWK93zORJVdvw/jlFkQxurngxN0gZPhm0Y8FScnAWxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrbwWfQFAAAAACJRIDe1ZUIvdxDz/0bjzvxoYWx8h8PSVBbYglHF/tQ9WLQ+7AEAAAEBKxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrYAAAA="
{
  "inputs": [
    {
      "has_utxo": true,
      "is_final": false,
      "next": "updater"
    }
  ],
  "estimated_vsize": 142,
  "estimated_feerate": 0.00004288,
  "fee": 0.00000609,
  "next": "updater"
}

Notice in this example we passed in 4000 for the feerate but the resulting transaction has a feerate of 4288. (609sats /142vbytes).

Just to confirm my suspicions because the psbt was not signed, I signed the psbt and analyzed that and it has the same feerate:

(bumpchannelopen) orangepi@orangepizero3:~/code/bumpchannelopen$ l1-cli signpsbt -k psbt="cHNidP8BAH0CAAAAAar1TjGVVY7r/qjT0r4lYg38aayDOaXIuZgul1Y66U/GAQAAAAD9////AugDAAAAAAAAFgAUROffcsiurCRhU91VG4nihgzhnt7Rl+YFAAAAACJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cK7QEAAAABAN0CAAAAApcQpKrQTQNBo9La5a0Z5HeUCeE7oB7kqsvdr43VunvFAAAAAAD9////UcFd/1/8N9met9j2Zluqbpbcgs9vknrAVRa2SxKrF+8BAAAAAP3///8D4MgQAAAAAAAiACDTzWK93zORJVdvw/jlFkQxurngxN0gZPhm0Y8FScnAWxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrbwWfQFAAAAACJRIDe1ZUIvdxDz/0bjzvxoYWx8h8PSVBbYglHF/tQ9WLQ+7AEAAAEBKxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrYAAAA="
{
   "signed_psbt": "cHNidP8BAH0CAAAAAar1TjGVVY7r/qjT0r4lYg38aayDOaXIuZgul1Y66U/GAQAAAAD9////AugDAAAAAAAAFgAUROffcsiurCRhU91VG4nihgzhnt7Rl+YFAAAAACJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cK7QEAAAABAN0CAAAAApcQpKrQTQNBo9La5a0Z5HeUCeE7oB7kqsvdr43VunvFAAAAAAD9////UcFd/1/8N9met9j2Zluqbpbcgs9vknrAVRa2SxKrF+8BAAAAAP3///8D4MgQAAAAAAAiACDTzWK93zORJVdvw/jlFkQxurngxN0gZPhm0Y8FScnAWxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrbwWfQFAAAAACJRIDe1ZUIvdxDz/0bjzvxoYWx8h8PSVBbYglHF/tQ9WLQ+7AEAAAEBKxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrYBE0DXd66xbOCoYPqkOm2bGmUqqSs4WOFffZzaxvSiRie8wOkLkAe1BiH4oGtTidYqwfk2CwhjUNP21BLUvr2oiWjkIRaUD6bgcRupDFHl0RUuseZHanCE291EnkoQeMsh3lGt6AkAqYonhAAAAAAAIgID8eWyxQ0uKq30WOOlzX7DmdwBDEu2+iewpT2S57L30P4IROffcgYAAAAAIQcHdR1kfAtqWffGt+GgcGjBRq0r9RilBFQLs8NtVy9ZcwkAEPKQtAsAAAAA"
}
(bumpchannelopen) orangepi@orangepizero3:~/code/bumpchannelopen$ bt-cli analyzepsbt "cHNidP8BAH0CAAAAAar1TjGVVY7r/qjT0r4lYg38aayDOaXIuZgul1Y66U/GAQAAAAD9////AugDAAAAAAAAFgAUROffcsiurCRhU91VG4nihgzhnt7Rl+YFAAAAACJRIFNlGAzKbzFe6oo7WeZTKFgph1a2wlScqWeiG3WTF+cK7QEAAAABAN0CAAAAApcQpKrQTQNBo9La5a0Z5HeUCeE7oB7kqsvdr43VunvFAAAAAAD9////UcFd/1/8N9met9j2Zluqbpbcgs9vknrAVRa2SxKrF+8BAAAAAP3///8D4MgQAAAAAAAiACDTzWK93zORJVdvw/jlFkQxurngxN0gZPhm0Y8FScnAWxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrbwWfQFAAAAACJRIDe1ZUIvdxDz/0bjzvxoYWx8h8PSVBbYglHF/tQ9WLQ+7AEAAAEBKxqe5gUAAAAAIlEgaEpiFZKtU0OuIk5eQJ+S7m1r56GN479Kr3pwt1kaQrYBE0DXd66xbOCoYPqkOm2bGmUqqSs4WOFffZzaxvSiRie8wOkLkAe1BiH4oGtTidYqwfk2CwhjUNP21BLUvr2oiWjkIRaUD6bgcRupDFHl0RUuseZHanCE291EnkoQeMsh3lGt6AkAqYonhAAAAAAAIgID8eWyxQ0uKq30WOOlzX7DmdwBDEu2+iewpT2S57L30P4IROffcgYAAAAAIQcHdR1kfAtqWffGt+GgcGjBRq0r9RilBFQLs8NtVy9ZcwkAEPKQtAsAAAAA"
{
  "inputs": [
    {
      "has_utxo": true,
      "is_final": false,
      "next": "finalizer"
    }
  ],
  "estimated_vsize": 142,
  "estimated_feerate": 0.00004288,
  "fee": 0.00000609,
  "next": "finalizer"
}

Again, 609 sats / 142 vbytes = 4288 feerate

Would appreciate any clarification on how txprepare handles the feerate argument and whether this is an intended behavior or a bug.

@Lagrang3
Copy link
Collaborator

It seems the feerate parameter is not well documented.
lightningd/feerate.c parses the feerate parameters as one of the following strings:
"slow", "normal", "urgent", "minimum" which are relevant for txprepare, besides other cases
"opening", "mutual_close", "penalty", "unilateral_close", "unilateral_anchor_close" that are specific
for internal purposes.
It can also be input as a number of blocks, eg. "Xblocks" where "X" is an integer.
And it can be input as a number of sats per unit of kweight "Xperkw",
or sats per kbytes "Xperkb", where "X" is an integer.
If no unit is specified, the "perkb" is assumed.

@rustyrussell rustyrussell self-assigned this Mar 27, 2025
@rustyrussell rustyrussell added this to the v25.05 milestone Mar 27, 2025
@rustyrussell
Copy link
Contributor

OK, this is actually quite well documented, but perhaps not easy to find?

https://docs.corelightning.org/reference/feerates#notes

But indeed, our estimator is off.

whitslack added a commit to whitslack/lightning that referenced this issue Apr 14, 2025
utxo_spend_weight(…) was assuming that witnesses always include a pubkey, but
that is not true of Taproot (key-path) spends. The effect was that wallet-
funded commands (e.g., withdraw, fundchannel, multifundchannel, etc.) were
overpaying on fees due to calculating Taproot input weights as 34 sipa greater
than they actually are.

Also as a consequence of the miscalculation, users who carefully select a set of
UTxOs to spend so as to avoid producing a change output would experience an
error when trying to spend them. As an example, I tried to spend a tiny 1390-sat
P2TR output to a new 1275-sat output at minimum feerate. The actual weight of
the transaction is 451 sipa, so the 115-sat fee yields a feerate of 255 sat/kw.
However, CLN refuses the transaction, claiming:

    {
       "code": 301,
       "message": "Could not afford 1275sat using UTXOs totalling 1390sat with weight 485 at feerate 253"
    }

The reported weight of 485 sipa is wrong, as it assumes the input will include a
34-sipa public key when in fact it will not.

This commit amends the bitcoin_tx_simple_input_witness_weight() and
bitcoin_tx_simple_input_weight(…) functions to take a parameter specifying the
scriptPubKey type of the spend. The implementation of the former is corrected so
that P2TR spends are not charged for a public key that they actually lack.

An enum type is introduced to enumerate the known scriptPubKey types, and a new
scriptpubkey_type(…) utility function is implemented to discern the type of a
given scriptPubKey. The existing is_known_scripttype(…) function is
reimplemented as an inline wrapper around the new function.

The default_lease_rates(…) function in plugins/funder_policy.c is amended so as
to state explicitly an assumption that it has been making: that the two inputs
it assumes for its default max weight calculation will not be Taproot inputs.

Fixes: ElementsProject#8164
Changelog-Fixed: P2TR inputs are assessed the correct weight in fee calculations.
@whitslack whitslack linked a pull request Apr 14, 2025 that will close this issue
4 tasks
@rustyrussell rustyrussell linked a pull request Apr 14, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants