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: bitcoin transaction fee assessment #557

Merged
merged 15 commits into from
Sep 24, 2024

Conversation

djordon
Copy link
Contributor

@djordon djordon commented Sep 24, 2024

Description

This is part of #552 but it does not close it.

This stuff will likely move to the signer crate, just not right in this PR.

Changes

  • Make sure sbtc crate integration tests are run in CI.
  • Use bitcoin-core v25 in CI.
  • Add a function to the BitcoinCoreClient for fetching transaction data that includes the transaction's fee. This new function, get_tx_info uses bitcoin-core's getrawtransaction RPC with a verbosity of 2, which is available in v25 and later.
  • Remove the in_active_chain field from the GetTxResponse struct. It was only ever populated when the BlockHash was given as input, and we have another function that is better now.
  • Add functions that apportioning the bitcoin fee for a deposit or withdrawal request after it has been confirmed. It follows [Feature]: Charge fees based on size #182, except it uses weight instead of size.

Testing Information

This PR adds an integration test for the new bitcoin-core client method and unit tests for the functions that apportion request fees. It also ensures integration tests in the sbtc crate get run.

Checklist:

  • I have performed a self-review of my code

@djordon djordon added transaction library Common library for handling transaction manipulation. sbtc bootstrap signer The sBTC Bootstrap Signer. labels Sep 24, 2024
Copy link
Member

@cylewitruk cylewitruk left a comment

Choose a reason for hiding this comment

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

Few questions, comments and suggestions :)

sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/src/rpc.rs Outdated Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
docker-compose.test.yml Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/src/rpc.rs Show resolved Hide resolved
sbtc/tests/integration/validation.rs Show resolved Hide resolved
Copy link
Member

@cylewitruk cylewitruk left a comment

Choose a reason for hiding this comment

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

Looks good!

@djordon djordon merged commit 5621236 into main Sep 24, 2024
3 checks passed
@djordon djordon deleted the 552-bitcoin-transaction-fee-assessment branch September 24, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sbtc bootstrap signer The sBTC Bootstrap Signer. transaction library Common library for handling transaction manipulation.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants