Repository navigation
mobile: support exact miner fee for refund transactions - #7
Conversation
|
Holding this for v13.3.0 rather than the
Once #10 is in, the simplest route for the exact refund fee is a |
|
@shubhamkmr04 could you rebase this? |
7dc89c8 to
169cc18
Compare
|
Rebased onto |
kaloudis
left a comment
There was a problem hiding this comment.
One bug: an exact minerFee at or above the lockup value produces an unbroadcastable transaction with a nil error. Details inline.
| }, | ||
| }, | ||
| boltz.Fee{SatsPerVbyte: &satPerVbyte}, | ||
| swapFee(feeRate, minerFee), |
There was a problem hiding this comment.
With a sats fee, nothing checks minerFee against the lockup value, and the Results from boltz.ConstructTransaction are discarded on line 234.
In boltz-client (pkg/boltz/transaction.go:134-140, same in v2.9.0 and v2.12.5), when the value is less than the fee, getOutValues only records value less than fee in Results and skips the output. constructBtcTransaction then builds a transaction with no outputs, and for a non-cooperative refund it signs the input and returns err == nil. When the fee equals the value, the check passes and a 0-sat output is added.
Reproduced on this branch with the existing newRefundFixture (100000 sat lockup), non-cooperative:
minerFee |
err |
outputs |
|---|---|---|
| 100000 | nil | 1 output, value 0 |
| 150000 | nil | none |
Neither transaction relays (bad-txns-vout-empty / dust), and CreateRefundTransaction passes the mempool.space rejection back with no hint of the cause. Before this PR the refund only took a fee rate, so this needed an absurd rate.
Suggested fix:
- Keep the results (
refundTransaction, results, err := ...) and returnresults[id].Errwhen it is set. - Reject a
minerFeethat leaves the output below dust (330 sat for P2TR, 294 for P2WPKH), sinceResultshas no error for the equal-value case.
A test calling BuildRefundTransaction with minerFee of 100000 and 150000 against the fixture, expecting an error, would cover both cases.
There was a problem hiding this comment.
Fixed in 08c2b5f. Both builders now return the error ConstructTransaction records in results, and reject a transaction with no output or one below dust (mempool.IsDust at the default relay fee, so 330/294). Tests cover 99800, 100000 and 150000 against the fixture.
| }, | ||
| }, | ||
| fee, | ||
| swapFee(feeRate, minerFee), |
There was a problem hiding this comment.
Same issue here: results from ConstructTransaction (line 169) is discarded. If Boltz refuses the cooperative claim, boltz-client falls back to a non-cooperative claim, which can return the same empty-output or 0-value transaction with a nil error when minerFee is at or above the lockup value.
There was a problem hiding this comment.
Fixed in 08c2b5f with the same check. A test claims against a host that refuses to co-sign, with minerFee of 100000 and 150000, and expects an error.
| // BuildRefundTransaction. | ||
| func CreateRefundTransaction(endpoint string, id string, claimLeaf string, refundLeaf string, transactionHex string, privateKey string, servicePubKey string, feeRate int32, timeoutBlockHeight int32, destinationAddress string, lockupAddress string, cooperative bool, isTestnet bool) (string, error) { | ||
| txHex, err := BuildRefundTransaction(endpoint, id, claimLeaf, refundLeaf, transactionHex, privateKey, servicePubKey, feeRate, timeoutBlockHeight, destinationAddress, lockupAddress, cooperative, legacyNetworkName(isTestnet)) | ||
| func CreateRefundTransaction(endpoint string, id string, claimLeaf string, refundLeaf string, transactionHex string, privateKey string, servicePubKey string, feeRate int32, minerFee int32, timeoutBlockHeight int32, destinationAddress string, lockupAddress string, cooperative bool, isTestnet bool) (string, error) { |
There was a problem hiding this comment.
This adds minerFee to CreateRefundTransaction's signature, but the doc comment says this wrapper is kept for app builds that predate BuildRefundTransaction. The current Zeus app still calls it with the old 13 arguments:
ios/LncMobile/LncModule.mm:343(LndmobileCreateRefundTransaction(..., feeRate, timeoutBlockHeight, ...))android/app/src/main/java/com/zeus/lnc-rn/LncModule.kt:269(Lndmobile.createRefundTransaction(...))
Both stop compiling once this build is bundled, unless they are updated at the same time. CreateReverseClaimTransaction keeps its signature, so the refund wrapper should do the same: drop the new parameter here and pass 0 for minerFee to BuildRefundTransaction, which keeps the fee-rate behaviour. Only BuildRefundTransaction needs the new argument.
af75a4a
into
ZeusLN:lnd-v0.21.4-beta-zeus.1
minerFeetoBuildRefundTransaction, so a refund can pay an exact fee in sats. 0 keeps the fee rate.CreateRefundTransactionkeeps its signature and passes 0.