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
Smarter CF tx change #27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack 2d82d3b
2376f1f
to
cf741a0
Compare
f00bed2
to
7659c04
Compare
Amended with a version that decides whether to create a new fb coin not based on Vb but based on a "minimum coin value". |
7659c04
to
45f3d79
Compare
Model/statemachine.py
Outdated
@@ -383,6 +379,14 @@ def grab_coins_2(self, block_height): | |||
total_to_consume = total_unprocessed + total_unallocated + total_negligible | |||
return total_to_consume, num_inputs | |||
|
|||
def min_fbcoin_value(self, height): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very similar to the StateMachine._is_negligible()
value. In that I put the coin should increase the cancel by at least 10 sats/vByte. Do you think 5 is sufficient? I don't know. They should be consistent I think. Also, I think if Vm
is much smaller, that is also a valid min value for a feebumping coin. So I'd use
min(Vm, int(feerate * P2WPKH_INPUT_SIZE + self._feerate_to_fee(5, "cancel", 0))
.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very similar to the StateMachine._is_negligible() value. In that I put the coin should increase the cancel by at least 10 sats/vByte. Do you think 5 is sufficient? I don't know.
Wow, then is_negligible is way too high in my opinion. For a reserve feerate of say 600sat/vbyte (2017 levels), this is coin at 64240 + 12290 = 76530sats (~38$ at 50k$/BTC) that you consider negligible. This is unreasonable.
Same computation with a feerate bump of 5sat/vb is 64240 + 6145 = 70385sats (~35$) which IMO is clearly enough. Keep in mind that at a large but reasonable feerate of 100sat/vb this would be a bump of more than 48sat/vb!
They should be consistent I think.
Absolutely not i think.
To create them we should be conservative to avoid having to consolidate them and "bumps by at least 5sat/vb at the reserve feerate" is a reasonable value IMO (happy to bikeshed).
However, removing them from an allocated vault reserve is a dangerous operation that we ought to avoid unless it's absolutely necessary. So i'd say "bumps by at least 1sat/vb at the reserve feerate" but it's still a very high value and one based on recent fee estimates might be more reasonable.
Of course if it's not allocated then it's another matter: we can keep the 5sats/vb and consolidate it whenever the feerate is low.
Also, I think if Vm is much smaller
If it is then we have a bigger issue lol
Fixed the code, results are even worse now... |
45f3d79
to
9caa5a0
Compare
25a4202
to
a9c6403
Compare
Rebased on master, this is now a single small commit |
a9c6403
to
ae38542
Compare
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
ae38542
to
17b9fb1
Compare
So i fixed the logic and this (almost) doesn't make this worse anymore. |
No description provided.