Skip to content

fix: swap allowAbsurdFees for maxFeeRate for Dash Core v18#90

Merged
shumkov merged 2 commits into
dashpay:masterfrom
dashhive:fix-absurd-maxfeerate
Sep 19, 2022
Merged

fix: swap allowAbsurdFees for maxFeeRate for Dash Core v18#90
shumkov merged 2 commits into
dashpay:masterfrom
dashhive:fix-absurd-maxfeerate

Conversation

@coolaj86

@coolaj86 coolaj86 commented Sep 14, 2022

Copy link
Copy Markdown

Feedback wanted: What should the default maxFeeRate be?

Re: dashpay/insight-ui#75

pshenmic
pshenmic previously approved these changes Sep 14, 2022

@pshenmic pshenmic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Comment thread lib/services/dashd.js Outdated

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@shumkov shumkov merged commit 95e1606 into dashpay:master Sep 19, 2022
@coolaj86

coolaj86 commented Sep 21, 2022

Copy link
Copy Markdown
Author

This PR is incomplete.

It took a few days for me to set up a full node to test (33gb of blocks and 50gb of indexes).

Notes for follow up:

node_modules/@dashevo/dashd-rpc/lib/index.js:

- sendRawTransaction: 'str bool bool',
+ sendRawTransaction: 'str float bool',

Re: dashpay/dashd-rpc#50

Even after that change I still get:

Error: "absurdly-high-fee, 383 > 373 (code 256). Code:-26"

Can't figure out where it's coming from.

@coolaj86

coolaj86 commented Sep 21, 2022

Copy link
Copy Markdown
Author

I updated this from feedback on discord that maxFeeRate was a float, but I'm pretty sure it's supposed to be an int.

When I change from 0.00001000 to 1000 that error goes away.

Also need to investigate @dashevo/dashcore-node/lib/services/web.js for socket io stuff...

@shumkov

shumkov commented Sep 21, 2022

Copy link
Copy Markdown
Collaborator

@coolaj86 so we need one more PR?

@coolaj86

Copy link
Copy Markdown
Author

All related PRs have been opened.

I've tested locally going over and under the maximum fee.

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.

4 participants