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(esplora): include previous TxOuts for fee calculation #1308

Merged
merged 1 commit into from Feb 5, 2024

Conversation

LagginTimes
Copy link
Contributor

@LagginTimes LagginTimes commented Jan 31, 2024

Description

Partially implements #1265.

The previous TxOut for transactions received from an external wallet are added as floating TxOuts to TxGraph to allow for fee calculation.

Notes to the reviewers

Currently only the esplora portion of #1265 has been implemented.
The electrum portion will potentially be done in a new PR, as discussed on the 1/30/24 Lib call.

Checklists

To Do:

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

Looking good! Just some small changes.

crates/esplora/src/async_ext.rs Outdated Show resolved Hide resolved
crates/esplora/tests/async_ext.rs Outdated Show resolved Hide resolved
crates/esplora/tests/async_ext.rs Outdated Show resolved Hide resolved
crates/esplora/tests/blocking_ext.rs Outdated Show resolved Hide resolved
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK 4990595

Although I do not like the use of .for_each (as mentioned in #1308 (comment)), I also do not think it should be a blocker.

The previous `TxOut` for transactions received from an external
wallet are added as floating `TxOut`s to `TxGraph` to allow for
fee calculation.
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

re-ACK 552f11c

Copy link
Member

@danielabrozzoni danielabrozzoni left a comment

Choose a reason for hiding this comment

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

ACK 552f11c

@danielabrozzoni danielabrozzoni merged commit 7aca884 into bitcoindevkit:master Feb 5, 2024
12 checks passed
@notmandatory notmandatory added this to the 1.0.0-alpha milestone Feb 5, 2024
@notmandatory notmandatory mentioned this pull request Feb 14, 2024
23 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants