Reduce implementation code inside CScript #12885

pull sipa wants to merge 4 commits into bitcoin:master from sipa:201803_reducescript changing 7 files +106 −118

sipa commented at 4:20 PM on April 4, 2018: member

This PR moves FindAndDelete and GetOp2 out of CScript (the first is only used inside the interpreter and moved there, the second does not actually depend on any script specifics and works on any vector). Furthermore, all non-const-iterator versions of GetOp are replaced by const ones, removing a number of methods in the process.

The longer term goal here is making the script interpreter independent from the CScript representation.

Note for reviewers: both FindAndDelete and GetScriptOp are consensus critical.
fanquake added the label Validation on Apr 4, 2018
in src/script/script.h:506 in 649b82a7a5 outdated
```
 503 |  
 504 |      bool GetOp(const_iterator& pc, opcodetype& opcodeRet) const
 505 |      {
 506 | -        return GetOp2(pc, opcodeRet, nullptr);
 507 | +        return GetScriptOp(pc, end(), opcodeRet, nullptr);
 508 |      }
```
Empact commented at 1:59 AM on April 5, 2018:

Could also move the GetOp definitions and make GetScriptOp static.

sipa commented at 11:53 PM on April 5, 2018:

That defeats the purpose. Longer term is needs to be accessible from the interpreter code.
promag commented at 11:58 PM on April 5, 2018: member

Refactor makes sense and looks good to me.

utACK 649b82a.
Empact commented at 8:15 AM on April 6, 2018: member

utACK 649b82a
ajtowns commented at 9:02 AM on April 12, 2018: contributor

ACK 649b82a7a58cf1708a4b7134e36e06ea70bdb710

Seems like CScript::Find() could be done away with completely as well.
Empact commented at 8:50 PM on April 12, 2018: member

@ajtowns yep that was true independent of this PR, opened #12969 to remove
ajtowns cross-referenced this on Apr 13, 2018 from issue Drop dead code CScript::Find by Empact
MarcoFalke commented at 1:22 PM on April 13, 2018: member

Needs rebase
Make iterators in CScript::FindAndDelete const 2fb168b55d
Delete unused non-const-iterator CSCript::GetOp overloads 33a8ecfbce
[MOVEONLY] Move CSCript::FindAndDelete to interpreter 6a7456ad60
[MOVEONLY] Turn CScript::GetOp2 into a function and move to cpp 54a5a21158
sipa force-pushed on Apr 13, 2018
sipa commented at 4:12 PM on April 13, 2018: member

Rebased.
ajtowns commented at 7:09 AM on April 16, 2018: contributor

ACK 54a5a21158f740990048b1d43c641630744cf3ee
laanwj merged this on Apr 23, 2018
laanwj closed this on Apr 23, 2018
laanwj referenced this in commit a49381dfa3 on Apr 23, 2018
Bushstar cross-referenced this on Apr 24, 2018 from issue commits from bitcoin/master by Bushstar
jasonbcox referenced this in commit c023ad5f28 on Sep 27, 2019
jonspock referenced this in commit 3193c29eab on Dec 24, 2019
jonspock referenced this in commit 49f5320ede on Dec 24, 2019
jonspock referenced this in commit 024933225c on Dec 24, 2019
jonspock referenced this in commit 6378fdabf3 on Dec 24, 2019
jonspock referenced this in commit 353cca3251 on Dec 24, 2019
jonspock referenced this in commit 7d87155e40 on Dec 26, 2019
PastaPastaPasta referenced this in commit ee56bef96d on Nov 10, 2020
PastaPastaPasta referenced this in commit 3178d1eb6c on Nov 12, 2020
PastaPastaPasta referenced this in commit 032dca5983 on Nov 17, 2020
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#12969 Drop dead code CScript::Find