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

Add server-side check for insanely high fees. #63

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrick
Copy link
Member

@jrick jrick commented Feb 22, 2021

Goal is to prevent a situation like decred/dcrwallet#2000 from
happening again even if users are running the buggy client code.

@@ -219,6 +219,11 @@ func feeForSerializeSize(relayFeePerKb int64, txSerializeSize int) int64 {
return fee
}

func paysHighFees(relayFeePerKb, fee int64, txSerializeSize int) bool {
maxFee := feeForSerializeSize(50*relayFeePerKb, txSerializeSize)
Copy link
Member Author

Choose a reason for hiding this comment

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

This value (50x over the standard fee rate) may need tweaking. This same function in dcrwallet defines high fees as paying over 1000x the standard rate.

We need to be cautious setting this too low, because it may disrupt the mixes at the lowest common denomination used for change mixing, as those transactions do not produce any change outputs.

Copy link
Member Author

Choose a reason for hiding this comment

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

After doing the math I believe this check should be for anything paying 104x or more than the required fee.

The lowest common denomination is 0.00262144, and the required fee to be contributed is 0.00002530.

0.00262144 / 0.00002530 = 103.6

So paying up to 103x can actually occur normally. But 104x or more indicates a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or 121 because the required fee without the change output is actually lower (0.0000217).

0.00262144 / 0.0000217 = 120.8

Goal is to prevent a situation like decred/dcrwallet#2000 from
happening again even if users are running the buggy client code.

While here, the fee calculation is fixed to consider the cost of input
scripts to redeem P2PKH outputs.  Even if this is not the case,
minimum fee requirements in dcrd use the same assumption.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant