Skip to content

Conversation

Grammar and readability fixups.
Clarifies "bip125-replaceable" helpstrings.
@glozow glozow added the Docs label Aug 22, 2022
@glozow glozow requested review from ariard and maflcko and removed request for ariard August 22, 2022 14:09
Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -255,7 +255,7 @@ static std::vector<RPCResult> MempoolEntryDescription()
{RPCResult{RPCResult::Type::STR_HEX, "transactionid", "parent transaction id"}}},
RPCResult{RPCResult::Type::ARR, "spentby", "unconfirmed transactions spending outputs from this transaction",
{RPCResult{RPCResult::Type::STR_HEX, "transactionid", "child transaction id"}}},
RPCResult{RPCResult::Type::BOOL, "bip125-replaceable", "Whether this transaction could be replaced due to BIP125 (replace-by-fee)"},
RPCResult{RPCResult::Type::BOOL, "bip125-replaceable", "Whether this transaction signals BIP125 replaceability or has an unconfirmed ancestor signaling BIP125 replaceability.\n"},
Copy link
Member

Choose a reason for hiding this comment

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

For reference, this change adds clarification around the #22209 misunderstanding

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, hopefully this can close that.

This comment was helpful background for me: #25575 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, depending on the user there might still be a need for just finding out the signal of this tx, not taking into account the ancestors.

So I think we could return two bools, or an enum, see #25575 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

So I think we could return two bools, or an enum, see #25575 (comment)

either would make sense imo 🤷

Copy link
Contributor

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

ACK 375ebad

Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

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

ACK 375ebad

Thanks for the fixups

@maflcko maflcko merged commit 92bb700 into bitcoin:master Aug 22, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 22, 2022
@glozow glozow deleted the 2022-08-125-fixup branch August 23, 2022 08:33
@bitcoin bitcoin locked and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants