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

Documentation regarding absolute_fee and fee_rate updated #957

Merged
merged 1 commit into from
May 28, 2023

Conversation

roy9495
Copy link
Contributor

@roy9495 roy9495 commented Apr 27, 2023

Description

This pr solves this issue #856
Updated the documentation for absolute fee and fee rate in BDK docs

All Submissions:

Bugfixes:

  • I'm linking the issue being fixed by this PR

@@ -190,6 +190,9 @@ impl<'a, D, Cs: CoinSelectionAlgorithm, Ctx: TxBuilderContext> TxBuilder<'a, D,
}

/// Set an absolute fee
/// The fee_absolute method refers to the absolute transaction fee in satoshis (sats).
/// If anyone set both the fee_absolute method and the fee_rate method,
Copy link
Member

Choose a reason for hiding this comment

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

grammar nit:

Suggested change
/// If anyone set both the fee_absolute method and the fee_rate method,
/// If anyone sets both the fee_absolute method and the fee_rate method,

@jonmarrs
Copy link
Contributor

jonmarrs commented May 4, 2023

While we are updating the absolute fee documentation, perhaps we should also update the fee rate documentation for the fee_rate method directly above the fee_absolute method.

Is the fee rate in satoshi/vbyte (e.g., 5.0 satoshi/vbyte)?

@jonmarrs
Copy link
Contributor

jonmarrs commented May 4, 2023

I've updated the documentation for the fee_rate method in this pull request: #969

@nondiremanuel nondiremanuel added documentation Improvements or additions to documentation BDK 1.0.0 labels May 6, 2023
@danielabrozzoni
Copy link
Member

Hi, can you please squash your commits together? Then I will merge this :)

@jonmarrs
Copy link
Contributor

I've squashed all of the commits together here: #969

@roy9495
Copy link
Contributor Author

roy9495 commented May 18, 2023

Hello, I have squashed my commits. Thank you.

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 17b3d65

@danielabrozzoni
Copy link
Member

Hey, can you please rebase on master to pick up the CI fixes?

@roy9495
Copy link
Contributor Author

roy9495 commented May 25, 2023

I rebased the branch. Thank you

@danielabrozzoni
Copy link
Member

@roy9495
Copy link
Contributor Author

roy9495 commented May 25, 2023

I signed the commits. Thank you

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 10b4b6c

@danielabrozzoni danielabrozzoni merged commit 9bc7fe8 into bitcoindevkit:master May 28, 2023
10 checks passed
@roy9495
Copy link
Contributor Author

roy9495 commented May 28, 2023

Thank You

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants