tarofreighter: redo fee accounting for PSBT send pkt after adding input #148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In this commit, we fix an existing TODO to ensure that the final packet we have has enough fees at the target fee level. Before this commit, when we added our own input, we would cause the final fee to undershoot the desired fee rate target. In order to fix this, we'll now compute the total weight and then required fee (at the fee rate) to meet our target. We'll then add this delta (`target-current`) to make sure we we're not undershooting in fees.
guggero
approved these changes
Sep 28, 2022
| return err | ||
| } | ||
|
|
||
| switch addrType { |
There was a problem hiding this comment.
Just curious: why aren't we using the txOut.PkScript here and do the same switch as above, but with outputs instead of inputs?
| feeDelta := int64(requiredFee) - currentFee | ||
| s.SendPkt.UnsignedTx.TxOut[lastIdx].Value += feeDelta | ||
|
|
||
| log.Infof("Adjusting send pkt by factor of %v from %v sats to %v sats", |
There was a problem hiding this comment.
nit: not a factor but a delta. Also, one is a uint64 and the other is an Amount, so this renders as from 9800 sats to 0.00012662 BTC sats
jharveyb
reviewed
Sep 28, 2022
| lastIdx := len(s.SendPkt.UnsignedTx.TxOut) - 1 | ||
| currentFee := inputAmt - outputAmt | ||
| feeDelta := int64(requiredFee) - currentFee | ||
| s.SendPkt.UnsignedTx.TxOut[lastIdx].Value += feeDelta |
There was a problem hiding this comment.
Thought the same at first too. But delta will be negative if we're overpaying and positive if we're underpaying. So adding is correct.
Edit: Never mind, brain fart. You're totally right. We need to decrease the change output value in order to increase the fee 🤦♂️
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In this commit, we fix an existing TODO to ensure that the final packet
we have has enough fees at the target fee level. Before this commit,
when we added our own input, we would cause the final fee to undershoot
the desired fee rate target. In order to fix this, we'll now compute the
total weight and then required fee (at the fee rate) to meet our target.
We'll then add this delta (
target-current) to make sure we we're notundershooting in fees.