Fix CI #10
Closed
Fix CI #10
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
The function has a lower bound defined by a `MIN_FEE` constant, which can only apply to full transactions. Usually we want to calculate the weight of a whole transaction, but sometimes we do want to calculate, for example, the weight of a transaction output.
We should fail if a funding transaction change output cannot be created without making a loss (cost greater than value). Currently, this logic does not take into account how much it will cost to spend the output. We should include that in the future.
Before this patch, the extra fee reserve was expected when building the funding transaction, but it was not included in the target amount that we pass to `get_utxos_for_amount`.
Before this patch, we were including the weight of the funding transaction[^1] when calculating the `amount` that we pass to `get_utxos_for_amount`. This was fine, but the weight of the funding transaction directly impacts the transaction fee that the consumer of `rust-dlc` will pay as soon as the channel is opened on-chain. This means that, before this patch, the bulk of the transaction fee was implicit in the amount that the consumer was selecting coins for. That behaviour is a bit surprising and it obfuscates the transaction fee to be paid, which is important information to be shown to the end user. Therefore, this patch adds a `base_weight_wu` which consumers must use to calculate the funding transaction fee during coin selection. [^1]: It's not correct, but the intention was there.
Inspired by this patch[^1] in `master`. Since in the previous patch the `FUND_TX_BASE_WEIGHT` is passed explicit to the implementer of `get_utxos_for_amount`, we can exclude that from the target `amount` that we pass to `get_utxos_for_amount`. Besides the party's collateral and all the fees related to opening a _channel_, the only thing other that we need to take into account is the transaction fee for the CET or the refund transaction. The base weight can be split in half between the two parties, and each party then has to pay for their own (possible) output on the CET or refund transaction. When constructing the fund transaction itself, we were also forgetting to add the `8 + 1` bytes of each CET or refund transaction party outputs. [^1]: p2pderivatives@c4a0f88.
We are obviously still in the LN-DLC branch, but the buffer transaction is smaller for vanilla DLC channels. This change is only needed because we are not back on `master`.
This won't be a problem once we move to `p2pderivatives/master`.
holzeis
approved these changes
Feb 22, 2024
| @@ -2,7 +2,7 @@ | |||
|
|
|||
| set -e | |||
|
|
|||
| CONTRACT_TEST_FILES=("Offered" "Accepted" "Confirmed" "Confirmed1" "Signed" "Signed1" "PreClosed" "Closed") | |||
There was a problem hiding this comment.
❓ Why did you remove the Offered, Accepted and Confirmed test cases?
luckysori
force-pushed
the
feat/display-fees-in-10101
branch
2 times, most recently
from
3cc980e
to
1ab4bf5
Compare
|
Not a priority atm, and it was annoying. |
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.
No description provided.