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

Fee Rates #320

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open

Fee Rates #320

wants to merge 8 commits into from

Conversation

Talej
Copy link

@Talej Talej commented Nov 9, 2023

It first tries to fetch fee rate from bb mempool, then mempool.space and finally if they fail it falls back to using bitcoin core.

I have implemented it to use confTarget rather than priority as the priorities can be inferred from the confTarget value and it allows the functionality to be more backward compatible. E.g. we can easily modify any other method currently using confTarget to use this new approach going forward.

I do have a small modification in batching.sh that I haven't committed as I noticed that seems to deal with both fee rate and confTarget already so wasn't sure if the change should be added there.

As well as updating the spend endpoint I have added in a new getfeerate endpoint in case we need to fetch the rates anywhere else.

Example:

curl -d '{"confTarget":6}' proxy:8888/getfeerate

@Talej
Copy link
Author

Talej commented Nov 9, 2023

Hm should the domains to query be loaded from a config rather than having bb and mempool.space hard coded?

@BullishNode
Copy link

Probably from configs is ideal

@BullishNode
Copy link

Also the feerate should be added to wasabi spend

@Talej
Copy link
Author

Talej commented Nov 11, 2023

Probably from configs is ideal

New commit added making the priorities configurable

@Kexkey
Copy link
Collaborator

Kexkey commented Nov 30, 2023

Nice PR!

Please move getfeeratefromurl, getpriorityfromconftarget and getfeerate functions from walletoperations.sh to bitcoin.sh.

In requesthandler.sh, rename getfeerate to bitcoin_getfeerate.

Also, we need to add action_bitcoin_getfeerate=watcher to cyphernodeconf_docker/templates/gatekeeper/api.properties for the request to reach the requesthandler.

It would be cool to have a small test-getfeerate.sh in proxy_docker/app/tests/.

Last thing is to add the new endpoint to doc/openapi/v0/cyphernode-api.yaml as well as doc/API.v0.md.

Thanks!!

@Talej
Copy link
Author

Talej commented Dec 1, 2023

Nice PR!

Please move getfeeratefromurl, getpriorityfromconftarget and getfeerate functions from walletoperations.sh to bitcoin.sh.

In requesthandler.sh, rename getfeerate to bitcoin_getfeerate.

Also, we need to add action_bitcoin_getfeerate=watcher to cyphernodeconf_docker/templates/gatekeeper/api.properties for the request to reach the requesthandler.

It would be cool to have a small test-getfeerate.sh in proxy_docker/app/tests/.

Last thing is to add the new endpoint to doc/openapi/v0/cyphernode-api.yaml as well as doc/API.v0.md.

Thanks!!

Should have covered all this off in the 2 commits above

@Kexkey
Copy link
Collaborator

Kexkey commented Dec 6, 2023

Can you make batchspend() (in batching.sh) use the new getfeerate function?

@Talej
Copy link
Author

Talej commented Dec 6, 2023

Can you make batchspend() (in batching.sh) use the new getfeerate function?

Added!

@Kexkey
Copy link
Collaborator

Kexkey commented Feb 22, 2024

I rebased on dev

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.

3 participants