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

feat: add advanced rbf guide #237

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gregdhill
Copy link
Member

@gregdhill gregdhill commented Jun 15, 2022

Signed-off-by: Gregory Hill [email protected]

Until we have programmatic support (and even after) we should have some documentation for Replace-By-Fee (RBF) to demonstrate how Vaults may increase fees to prevent liquidation after significant Bitcoin mempool congestion.

See also: https://github.com/BlockchainCommons/Learning-Bitcoin-from-the-Command-Line/blob/master/05_2_Resending_a_Transaction_with_RBF.md

UPDATE: automatic RBF functionality was implemented here

Signed-off-by: Gregory Hill <[email protected]>
@vercel
Copy link

vercel bot commented Jun 15, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
interbtc-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 9, 2023 4:40pm

# use those to create a new transaction
rawtx=$(bitcoin-cli createrawtransaction $rawinputs $rawoutputs)
# fund that tx with a better fee rate
fundedtx=$(bitcoin-cli fundrawtransaction $rawtx "{\"changeAddress\": \"$CHANGE_ADDRESS\", \"feeRate\": $FEE_RATE}" | jq -r .hex)
Copy link
Member

Choose a reason for hiding this comment

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

You are counting on all addresses in the wallet to have been registered to the parachain, which is most cases will be true, but it's still a risky assumption. If e.g. a non-empty wallet was used when first starting the vault this would not hold (we do tell vaults not to do this.. but still)

Copy link
Member Author

Choose a reason for hiding this comment

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

Theft reporting was removed here, addresses are no longer required to be registered.

docs/vault/guide.md Outdated Show resolved Hide resolved
docs/vault/guide.md Show resolved Hide resolved
docs/vault/guide.md Show resolved Hide resolved
@nud3l
Copy link
Member

nud3l commented May 29, 2023

@gregdhill could you address the comments so we can merge this please?

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