Docs: [policy] Remove outdated confusing comment #24305
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maflcko
changed the title
laanwj
reviewed
Feb 10, 2022
jonatack
reviewed
Feb 11, 2022
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
murchandamus
force-pushed
the
2022-02-improve-cfeerate-docs
branch
from
bdc35cf to
fd2fb45
Compare
|
Thanks, I've updated the PR to drop the square brackets and amend the commit message. I've also added the commas as suggested. |
murchandamus
force-pushed
the
2022-02-improve-cfeerate-docs
branch
from
fd2fb45 to
fff3c83
Compare
|
ACK WDYT about the following, while here diff
private:
- /** fee rate in sat/kvB (satoshis per 1000 virtualbytes) */
+ /** Fee rate in sat/kvB (satoshis per 1000 virtualbytes) */
CAmount nSatoshisPerK;
public:
- /** Fee rate of 0 satoshis per kB */
+ /** Fee rate of 0 satoshis per kvB */
CFeeRate() : nSatoshisPerK(0) { }
/**
* Return the fee in satoshis for the given size in bytes.
- * If the calculated fee would have fractional satoshis, then the returned fee will always be rounded up to the nearest satoshi.
+ * If the calculated fee would have fractional satoshis, then the
+ * returned fee will always be rounded up to the nearest satoshi.
*/
CAmount GetFee(uint32_t num_bytes) const; |
This comment described how the constructor of CFeeRate was previously indirectly used to parse fee rate arguments from RPCs. The command line input was actually in sat/vB but due to the use of AmountFromValue() it got converted to BTC/vB. In the constructor this could be rectified by creating a CFeeRate from that given value (in BTC/vB) and COIN as the transaction size, turning the input effectively to sat/vB. Since this usage pattern was removed from the codebase some months ago, the comment is now obsolete. Also: • Use vsize and vbyte instead of size and byte
murchandamus
force-pushed
the
2022-02-improve-cfeerate-docs
branch
from
fff3c83 to
e50a9be
Compare
|
Sure, @jonatack, let's do it. |
jonatack
reviewed
Feb 15, 2022
|
ACK e50a9be I'm not sure about using the term |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This comment described how the constructor of CFeeRate was previously indirectly used to parse fee rate arguments from RPCs. The command line input was actually in sat/vB but due to the use of AmountFromValue() it got converted to BTC/vB which then got rectified in the constructor by creating a CFeeRate from that given value and COIN as the transaction size. Since this usage pattern was removed from the codebase some months ago, the comment is now obsolete.