server/asset/doge: use feeConfs and reduce maxFeeBlocks #1965
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
chappjc
commented
Nov 18, 2022
server/asset/doge/doge.go
Outdated
| @@ -92,7 +92,8 @@ func NewBackend(configPath string, logger dex.Logger, network dex.Network) (asse | |||
| Net: network, | |||
| ChainParams: params, | |||
| Ports: ports, | |||
| DumbFeeEstimates: true, | |||
| DumbFeeEstimates: true, // dogecoind actually has estimatesmartfee though... | |||
chappjc
commented
Nov 18, 2022
| @@ -49,7 +49,7 @@ const ( | |||
| version = 0 | |||
| BipID = 3 | |||
| assetName = "doge" | |||
| feeConfs = 10 | |||
| feeConfs = 8 | |||
There was a problem hiding this comment.
The table I pasted there is still accurate even today, except for the high number at 3.
chappjc
commented
Nov 18, 2022
| @@ -14,7 +14,7 @@ import ( | |||
| "github.com/btcsuite/btcd/chaincfg" | |||
| ) | |||
|
|
|||
| var maxFeeBlocks = 50 | |||
There was a problem hiding this comment.
In practice, it hung and context times out with a remote node. This just needs to make an quick effort and fall back to the "no competition" rate it can't get 101 txns in 16 blocks or less.
chappjc
force-pushed
the
server-fee-confs-set
branch
from
654a3db
to
6f5fc53
Compare
chappjc
force-pushed
the
server-fee-confs-set
branch
from
6f5fc53
to
2e8d9b1
Compare
buck54321
approved these changes
Nov 20, 2022
JoeGruffins
approved these changes
Nov 21, 2022
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.
This resolves a couple minor bugs introduced in #1597
The
feeConfsarg that was previously used forestimatefeewas neglected when theFeeEstimatorconfig item was replaced with a more sophisticated(*Backend).estimateFeemethod. This setting was important for DOGE and BCH where a value of 1 almost always returns an estimate of -1 (failure), resulting in one of the median fee estimates. In the case it was the "hard" way that is extremely demanding in terms of RPCs to the node, making a remote dogecoind instance virtually infeasible.A second change is reducing the max blocks in the hard median fee estimator from 50 to 16, which is plenty because if it doesn't find 101 txns in 16 blocks it should just go with the "no competition" rate.