Skip to content
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

chaintopology: set a fee floor to avoid creating unrelayable txs. #1251

Conversation

rustyrussell
Copy link
Contributor

Naively, this would be 250 satoshi per sipa, but it's not since bitcoind's
fee calculation was not rewritten to deal with weight, but instead bolted
on using vbytes.

The resulting calculations made me cry; I dried my tears on the thorns
of BUILD_ASSERT (I know that makes no sense, but bear with me here as I'm
trying not to swear at my bitcoind colleagues right now).

Fixes: #1194
Signed-off-by: Rusty Russell [email protected]

Naively, this would be 250 satoshi per sipa, but it's not since bitcoind's
fee calculation was not rewritten to deal with weight, but instead bolted
on using vbytes.

The resulting calculations made me cry; I dried my tears on the thorns
of BUILD_ASSERT (I know that makes no sense, but bear with me here as I'm
trying not to swear at my bitcoind colleagues right now).

Fixes: ElementsProject#1194
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Contributor Author

I think this is the more general fix than #1247 in that it fixes all transactions we generate.

Copy link
Contributor

@ZmnSCPxj ZmnSCPxj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK overall, can see your "passion" (frustration) in the comments around and before feerate_floor

u32 feerate = satoshi_per_kw[i];

if (feerate < feerate_floor())
feerate = feerate_floor();
Copy link
Contributor

@ZmnSCPxj ZmnSCPxj Mar 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest adding a log_debug here that we are bumping the feerate up because hsyterical raisins. Or alternatively expand the existing log_debug message that we bumped it up and the actual feerate received from bitcoind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@ZmnSCPxj
Copy link
Contributor

ACK c059b8d

…floor.

But only if we're actually going to change the feerate, otherwise we'd
log every time.

Suggested-by: @ZmnSCPxj
Signed-off-by: Rusty Russell <[email protected]>
@ZmnSCPxj
Copy link
Contributor

ACK 1d3b056

@ZmnSCPxj
Copy link
Contributor

Amusing:
@rustyrussell wants to merge 2 commits into ElementsProject:master from rustyrussell:omg-bitcoind-vbytes-must-die-die-die

@cdecker
Copy link
Member

cdecker commented Mar 20, 2018

Fixes #1210

@cdecker
Copy link
Member

cdecker commented Mar 20, 2018

ACK 1d3b056

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants