Skip to content

Conversation

murchandamus
Copy link
Contributor

No description provided.

@maflcko
Copy link
Member

maflcko commented Nov 15, 2019

Oh, looks like it is still used in miner.cpp:

$ git grep COINBASE_FLA
src/miner.cpp:    txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce)) + COINBASE_FLAGS;
src/rpc/mining.cpp:    aux.pushKV("flags", HexStr(COINBASE_FLAGS.begin(), COINBASE_FLAGS.end()));
src/validation.h:extern CScript COINBASE_FLAGS;

@maflcko
Copy link
Member

maflcko commented Nov 15, 2019

So it might be better to move it to miner.h,cpp. It is confusing to have this sit in validation.h,cpp, where it appears to be an unused variable.

@murchandamus
Copy link
Contributor Author

Ah, my bad. Closing then, as I guess this falls into the refactors section rather than a simple deletion.

@laanwj
Copy link
Member

laanwj commented Nov 19, 2019

But what is the point of it, if it's never assigned a value?

Looks like the last assignment was removed in d449772.

I suppose it's kept around for future signalling purposes, but yes, moving it to mining at least would make sense.

@jnewbery
Copy link
Contributor

Concept ACK on removal. COINBASE_FLAGS has not been used since BIP 16/17 signalling.

I think @narula is planning to pick this one up.

laanwj added a commit that referenced this pull request Nov 22, 2019
e9a27cf refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772 stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on #17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf
  MarcoFalke:
    ACK e9a27cf 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 22, 2019
e9a27cf refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772 stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on bitcoin#17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf
  MarcoFalke:
    ACK e9a27cf 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
e9a27cf refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772 stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on bitcoin#17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf
  MarcoFalke:
    ACK e9a27cf 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
vijaydasmp pushed a commit to vijaydasmp/dash that referenced this pull request Aug 30, 2021
e9a27cf refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772 stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on bitcoin#17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf
  MarcoFalke:
    ACK e9a27cf 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
PastaPastaPasta added a commit to dashpay/dash that referenced this pull request Aug 31, 2021
* Merge bitcoin#17519: rpc: Remove unused COINBASE_FLAGS

e9a27cf refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772 stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on bitcoin#17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf
  MarcoFalke:
    ACK e9a27cf 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334

* Update src/rpc/mining.cpp

Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>

Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 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.

5 participants