Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Improving the documentation for TxBuilder #856

Closed
prakharjain-slice opened this issue Feb 9, 2023 · 5 comments
Closed

Improving the documentation for TxBuilder #856

prakharjain-slice opened this issue Feb 9, 2023 · 5 comments
Assignees
@danielabrozzoni
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers module-wallet
Milestone
1.0.0-alpha.1

Comments

@prakharjain-slice
Copy link

Describe the enhancement
Documentation regarding absolute fee and fee rate is little unclear in BDK docs. Common concerns are:

  • Is absolute fee in sats or btc?
  • If we add both absolute fee and fee rate while building the transaction which will be honoured?

Use case
This documentation will help users in better understanding of Tx object and can optimise their transaction fees.
@prakharjain-slice prakharjain-slice added the new feature New feature or request label Feb 9, 2023
@danielabrozzoni danielabrozzoni added documentation Improvements or additions to documentation good first issue Good for newcomers and removed new feature New feature or request labels Feb 10, 2023
@notmandatory notmandatory moved this to Todo in BDK Maintenance Feb 13, 2023
@notmandatory
Copy link
Member

See also discussion in #823

@tusharxoxoxo tusharxoxoxo mentioned this issue Apr 12, 2023
Closed
3 tasks
@roy9495 roy9495 mentioned this issue Apr 27, 2023
Merged
3 tasks
@roy9495
Copy link
Contributor

roy9495 commented Apr 29, 2023

Can someone please review this #957

@notmandatory notmandatory added this to BDK May 4, 2023
@jonmarrs jonmarrs mentioned this issue May 4, 2023
Merged
3 tasks
@notmandatory notmandatory moved this to Todo in BDK May 6, 2023
@notmandatory notmandatory moved this from Todo to In Progress in BDK May 6, 2023
danielabrozzoni added a commit that referenced this issue May 28, 2023
@danielabrozzoni
Merge #957: Documentation regarding absolute_fee and fee_rate updated
9bc7fe8 
10b4b6c Documentations regarding absolute_fee and fee_rate updated (TATHAGATA ROY)

Pull request description:

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

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

  #### Bugfixes:

  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  danielabrozzoni:
    ACK 10b4b6c

Tree-SHA512: 5b5f360b742313443368dc0b75275c38888cb358545661f7d9c78f3290789c50ffa9830e18473055d8e3da544c7d9527b023a1acdb28eaf862b24c892b23e9c4
danielabrozzoni added a commit that referenced this issue Sep 19, 2023
@danielabrozzoni
Merge #969: Update documentation for fee_rate and fee_absolute method…
f95506b 
…s in tx_builder.rs

e6519e3 Enhance the documentation for the fee_rate and fee_absolute methods. (Jon Marrs)

Pull request description:

  ### Description

  This pr helps solve this issue: #856

  I added documentation to the fee_rate() method to describe the units as either satoshis/vbyte (sats/vbyte) or satoshis/kwu (sats/kwu), depending on the FeeRate type.

  I also added documentation to the fee_absolute() method to clarify that the fee is determined by whichever method (fee_rate or fee_absolute) was called last, as the FeePolicy is an enum, and FeeRate/FeeAmount are mutually exclusive.

  ### Notes to the reviewers

  I thought it would be helpful to provide documentation to alleviate confusion over the fee_rate method and the fee_absolute method.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)

  #### Bugfixes:

  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  danielabrozzoni:
    ACK e6519e3

Tree-SHA512: 59f62d1d1f8355e6353c6a2550e464732975c86c767648adc9143f2b3a9b894a90536a30a33e9de7efbe53f16392ec2e19008d884fb65ef037edae64a3cb6970
@15IITian
Copy link

15IITian commented Feb 26, 2024 •
edited
Loading

Is it appropriate to close this issue as IMO all of its tasks are completed.

@danielabrozzoni
Copy link
Member

#1159 still needs to be merged, but CI is broken

@nondiremanuel nondiremanuel added this to the 1.0.0-alpha milestone Feb 27, 2024
@nondiremanuel nondiremanuel moved this from In Progress to Needs Review in BDK Feb 27, 2024
@notmandatory notmandatory moved this from Todo to In Progress in BDK Maintenance Mar 19, 2024
@notmandatory notmandatory moved this from In Progress to Ready to Review in BDK Maintenance Mar 19, 2024
@notmandatory notmandatory removed this from BDK Mar 19, 2024
@notmandatory notmandatory modified the milestones: 1.0.0-alpha, 0.30.0 Mar 19, 2024
@notmandatory notmandatory removed this from the 0.30.0 milestone Oct 22, 2024
@notmandatory notmandatory added this to the 1.0.0-alpha.1 milestone Oct 22, 2024
@notmandatory
Copy link
Member

fixed by #957

@github-project-automation github-project-automation bot moved this from Ready to Review to Done in BDK Maintenance Oct 22, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees

@danielabrozzoni danielabrozzoni

Labels
documentation Improvements or additions to documentation good first issue Good for newcomers module-wallet
Projects
BDK Maintenance
Archived in project
Milestone
1.0.0-alpha.1
Development

No branches or pull requests
6 participants
@notmandatory @danielabrozzoni @nondiremanuel @roy9495 @prakharjain-slice @15IITian