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: track btc fees on success #4334

Merged
merged 3 commits into from Dec 12, 2023
Merged

feat: track btc fees on success #4334

merged 3 commits into from Dec 12, 2023

Conversation

kylezs
Copy link
Contributor

@kylezs kylezs commented Dec 8, 2023

Pull Request

Closes: PRO-1034

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works - did test the recorded fee equals that of the local explorer, and it matches.
  • I have updated documentation where appropriate.

Summary

We want to track how much fees the vault has used to pay for egress.

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

Attention: 52 lines in your changes are missing coverage. Please review.

Comparison is base (d92d824) 71% compared to head (c1070df) 71%.
Report is 2 commits behind head on main.

Files Patch % Lines
engine/src/btc/rpc.rs 0% 40 Missing ⚠️
engine/src/witness/btc/btc_deposits.rs 91% 7 Missing ⚠️
engine/src/witness/btc.rs 91% 3 Missing ⚠️
engine/src/btc/retry_rpc.rs 0% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #4334   +/-   ##
=====================================
- Coverage     71%     71%   -0%     
=====================================
  Files        390     390           
  Lines      66259   66293   +34     
  Branches   66259   66293   +34     
=====================================
- Hits       47292   47230   -62     
- Misses     16602   16684   +82     
- Partials    2365    2379   +14     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AlastairHolmes
Copy link
Contributor

This waiting for something?

@kylezs
Copy link
Contributor Author

kylezs commented Dec 11, 2023

@martin-chainflip was going to review

@kylezs kylezs force-pushed the feat/bitcon-fee-witness branch 4 times, most recently from 630ca60 to eb59185 Compare December 12, 2023 10:17
@kylezs kylezs force-pushed the feat/bitcon-fee-witness branch 2 times, most recently from e6de2dd to b0d63bb Compare December 12, 2023 10:58
// Don't care to write custom deserializer for this
#[serde(skip)]
pub version_hex: Option<Vec<u8>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you add this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Same below

@kylezs kylezs merged commit 92cbd6f into main Dec 12, 2023
41 checks passed
@kylezs kylezs deleted the feat/bitcon-fee-witness branch December 12, 2023 17:13
kylezs added a commit that referenced this pull request Dec 12, 2023
* feat: track btc fees on success

* feat: use verbose block

* doc: serde_json bug comment
syan095 added a commit that referenced this pull request Dec 18, 2023
…-timeout

* origin/main:
  feat: end to end network upgrade github action (#4274)
  feat: spec_version of PR is greater than spec version of current release (#4355)
  feat: enforce version is greater than release version on PRs to main (#4351)
  fix: btc witnesser test failing sometimes (#4353)
  fix: connections can become stale when reconnecting (#4310)
  chore: add `chainflip-rpc-node` systemd file 🚀 (#4352)
  feat: API Bins check SC compatibility (#4342)
  chore: update runtime spec checks ⛓️ (#4349)
  feat: Add version cmd to all bins (#4343)
  fix: changelog check 🤫 (#4348)
  chore: update docker tags 🐳 (#4347)
  chore: add runtime version check 👀 (#4344)
  feat: shave fees on ingress (#4335)
  pick/persa fixes (#4329)
  feat: track btc fees on success (#4334)
  ensure we dont create BTC transaction outputs below the bitcoin dust limit (#4340)
  fix: sweeping before withdrawal (#4337)
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.

None yet

3 participants