Skip to content

Conversation

@conscott
Copy link
Contributor

@conscott conscott commented Mar 23, 2018

BIP 22 - getblocktemplate specifies an optional flag, required if the transaction must be in the block.

Luke's implementation #936 did not include this flag, and it was later added to the help description in #3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a getblocktemplate call, so it seems logical to remove it from the help description.

If I am missing something or this is considered harmless - I can close the PR.

@conscott conscott force-pushed the remove_unused_gbt_field branch 2 times, most recently from b537924 to ae743d1 Compare March 23, 2018 09:32
@instagibbs
Copy link
Member

kicked travis, random timeout

@conscott conscott force-pushed the remove_unused_gbt_field branch 2 times, most recently from 127eae8 to 777515a Compare March 24, 2018 18:28
@conscott conscott force-pushed the remove_unused_gbt_field branch from 777515a to ac8a1d0 Compare March 26, 2018 10:02
@laanwj
Copy link
Member

laanwj commented Mar 27, 2018

Interesting, good catch.
Though personally I think it makes sense to describe this possible field if it is in BIP22. Both for consistency with our documentation and the BIP, and in case core would ever want to set this flag, you'd want client implementations to take it into account.
On the other hand that's probably very unlikely. Needs more discussion.

@jnewbery
Copy link
Contributor

Tested ACK ac8a1d0

@luke-jr
Copy link
Member

luke-jr commented Jun 12, 2018

Anyone writing a client should read BIP22 and consider all its possible fields. IMO either we should document potentially-relevant ones in help, or only refer to the BIP instead.

@maflcko
Copy link
Member

maflcko commented Jun 12, 2018

utACK ac8a1d0. It would be misleading to mention the field here and then ignore it without notice.

@DrahtBot
Copy link
Contributor

The last travis run for this pull request was 117 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

@DrahtBot DrahtBot closed this Jul 21, 2018
@DrahtBot DrahtBot reopened this Jul 21, 2018
@maflcko maflcko changed the title Remove field in getblocktemplate help that has never been used. doc: Remove field in getblocktemplate help that has never been used. Jul 21, 2018
@maflcko maflcko added the Docs label Jul 21, 2018
@maflcko maflcko merged commit ac8a1d0 into bitcoin:master Jul 22, 2018
maflcko pushed a commit that referenced this pull request Jul 22, 2018
…er been used.

ac8a1d0 [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott)

Pull request description:

  [BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block.

  Luke's implementation #936 did not include this flag, and it was later added to the help description in #3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description.

  If I am missing something or this is considered harmless - I can close the PR.

Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44
@conscott conscott deleted the remove_unused_gbt_field branch July 31, 2018 08:01
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request May 8, 2020
…er been used.

Summary:
ac8a1d0 [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott)

Pull request description:

  [BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block.

  Luke's implementation #936 did not include this flag, and it was later added to the help description in #3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description.

  If I am missing something or this is considered harmless - I can close the PR.

Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44

Backport of Core [[bitcoin/bitcoin#12764 | PR12764]]

Test Plan: `ninja check`

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D6015
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 17, 2020
…has never been used.

ac8a1d0 [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott)

Pull request description:

  [BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block.

  Luke's implementation dashpay#936 did not include this flag, and it was later added to the help description in dashpay#3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description.

  If I am missing something or this is considered harmless - I can close the PR.

Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants