refactor: Make CScriptVisitor stateless

promag commented at 10:06 am on May 4, 2020: member

CScriptVisitor was added in 1025440184ef100a22d07c7bb543ee45cf169d64 (#1357) and the visitor return type was never used. Now CScriptVisitor is stateless and CScript is the return type.

promag commented at 10:07 am on May 4, 2020: member

@sipa mind taking a look since you wrote this code?

MarcoFalke commented at 11:16 am on May 4, 2020: member

All of this code will go away anyway when we switch to C++17 in 5 months, except for the script << .. parts.

ACK on changing * to &, since that will be needed for my patch to switch to C++17. No opinion on changing the return type.

in src/script/standard.cpp:251 in ae0891e87b outdated

249-    explicit CScriptVisitor(CScript *scriptin) { script = scriptin; }
250+    explicit CScriptVisitor(CScript& scriptin) : script(scriptin) {}
251 
252-    bool operator()(const CNoDestination &dest) const {
253-        script->clear();
254-        return false;

promag commented at 11:19 am on May 4, 2020:

I’m curious why return false here.

MarcoFalke commented at 11:36 am on May 4, 2020:

Sometimes code is written to have a return type even though the return type is initially unused. Motivation for that could be that the initial implementation wants to accommodate future callers of the code, which may want to use the return type.

promag commented at 11:52 am on May 4, 2020:

Yeah, I think that motivation is long gone.

DrahtBot added the label Refactoring on May 4, 2020

MarcoFalke commented at 11:58 am on May 4, 2020: member

ACK ae0891e87b0aa0ce57a77dbd99555803a539d6ea, reviewed first commit with –word-diff and second commit with –word-diff-regex=. 🌲

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK ae0891e87b0aa0ce57a77dbd99555803a539d6ea, reviewed first commit with --word-diff and second commit with --word-diff-regex=. 🌲
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUichwwAgiZLmgANz5JYgG5YI/UqYLczqAGB1HgiAvykODXF0Ew7PowJt9DFLJE0
 8lM6Rfzy9n9086xvqItRf3jBH3l2IJ0Zp4AeSCUQV0t7s6sT3+vyqr7G6y1S+m8ai
 90RjiSADrutAyd69XY2TFcrwLWGGeA4Zk+JMt4dcUGVaGA1T2IveNkeBwG6N0TtVf
10I2DwYidq5Q620EGdchwrbkGInoXOoygBZ2Cths7MrNsaGl6XXOInJ6eM4azIqz04
11why88n1FCcFq0cakKbuQnzf7Llx6ZTs2X1lbPM2t8PfYa1jTjb6zC0xkW72si4tj
12BSps4CwmQb6bFpB7A2p/dTc6Int7+xubjlhRTXO6m3KcYvYeKBjdWzDhbxhCsgY9
13Q+FJGJ3pCT/aENy8OeYg+erxgl94bse/x75mALfDVoXqqOAYyI8Qg0Dvk2nYnMhL
14FC60WPXNqM9agXrNcWZdHzYWFcXoT1YzcReWHf/Whk4zT3iT2FGmVUKNh/HCvavs
15X90+LGrb
16=GIW/
17-----END PGP SIGNATURE-----

Timestamp of file with hash 9c646a620b75db224cc45898abb4e8eea4d622bbe4c774fba01d191da0fd0bde -

practicalswift commented at 2:08 pm on May 4, 2020: contributor

Concept ACK

MarcoFalke commented at 3:08 pm on May 4, 2020: member

Open-Close to re-run ci. See #15847 (comment)

MarcoFalke closed this on May 4, 2020

MarcoFalke reopened this on May 4, 2020

morucci referenced this in commit 7947350645 on May 10, 2020

promag commented at 11:59 pm on May 24, 2020: member

@sipa friendly ping.

sipa commented at 6:23 am on May 25, 2020: member

utACK ae0891e87b0aa0ce57a77dbd99555803a539d6ea

It could be made even shorter using script = CScript() << ...; statements.

promag force-pushed on May 25, 2020

promag commented at 10:31 am on May 25, 2020: member

It could be made even shorter using script = CScript() << ...; statements.

Done.

promag force-pushed on May 25, 2020

MarcoFalke commented at 11:42 am on May 26, 2020: member

ACK 1eb73933399dc8d9ff536fa668a98b03bd7d597b 🎰

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK 1eb73933399dc8d9ff536fa668a98b03bd7d597b 🎰
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUhSiQv/fgvq0c6RiAaVCFdQYJEw9rluU+T8RqlpmWsEO6X7Bv/1N27halQrg91Y
 8x0EzilW+nS+FUgMYfnm+QEU49Q7Tb1BgZa2xR6lUueS+koV01FzKJf1niDa6D9ll
 9CLvvQP10Cc02c4QZLM7vfrrIEHPRbzD+Nl5nPbRDMDwfGshsMOcfhV7t/7mI718i
10BssPWXqiCPPbLu7TZIiWwSZjcj3ISkbrfiPvpBuWkCQ11n5Da2C4vubcIY8D5uIZ
11fvH9s/Tp+Uu6VgyNWmwLfs9Mhq1HiuMxdoIWnXwz8XTp7xZgf+THEAfDbpoPQty0
12Nt4zH8bokN4jxVM9Ac162fZs3hw4jxqTS0zpjzDqp4YGun1CDkNgm/AVciL6UFoK
13aoXcBJmTe8NE24FGTpErtUIIVIdY+yf1oex8SkjzkCZ0i7v8HnlucdTjsmS/Za6K
14NkesEzQXgq0A7zBcsbtVlO29QgoxULvPIXxr/evnXI3JOYdsRvGlv+ibAVtdlgWy
15GwUy9f8h
16=OqF+
17-----END PGP SIGNATURE-----

Timestamp of file with hash d8f829f43c62cd26aff2b38a635ed9ae65308f0b2c3a17cbf8bbb9470285b962 -

sipa commented at 11:00 pm on June 5, 2020: member

utACK the current code.

Another suggestion - and feel free to ignore - given that the return type bool is now gone, why not use it for the script itself? The return type could be CScript directly.

refactor: Make CScriptVisitor stateless 3351c91ed4

promag force-pushed on Jun 5, 2020

promag commented at 11:46 pm on June 5, 2020: member

Yeah I saw it too - previous change made it obvious - but was lazy to do it. Updated, looks better.

promag renamed this:
~~refactor: Drop unused CScriptVisitor return type~~
refactor: Make CScriptVisitor stateless
on Jun 5, 2020

DrahtBot commented at 4:26 am on June 6, 2020: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

promag commented at 2:21 pm on June 15, 2020: member

@MarcoFalke @sipa should be an easy review.

MarcoFalke commented at 10:47 am on June 17, 2020: member

ACK 3351c91ed402895dcb4f803a29d2cac70ccfa8b4 🏤

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3ACK 3351c91ed402895dcb4f803a29d2cac70ccfa8b4 🏤
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUg8ygv/V2sqtDYeTS42806JVJrqjvyeXKqMUmYa9wTTJSt/Tqg5VwAf1P3NEwnW
 8pFdpfLWpJwrgFCIxO2yNZP1cwTY7JNYogFr8A223YuWVcKcr/nRibKr5IbY+jCcu
 9/TDA/t01fOWwtYr9hBC/g5I5nxjnAUR354f/gcHVYeXso3UnKBCyTim74Ddz9SKb
107UAsMM0rQ0VNvl0RgocrjtpyeE0V8ilbZI3y36bsDTlD1v1TSHyyyszC2zSobQ6b
11P1kt24Ig1qcLKy4HmFPq973AVXeksNDlG43WzXzLX9AV8I89G/u/0Nd8hzcki+yc
12IzcdBAMa6FZkGOZUPGKIFmmuUht4eDXXtd9gzwJDY9BzLZoZYp9PgNwPYSoWGzBr
13M3CTky/KyxbHcMZ6HXViRc8Bg3kdfEcz8TteJQ45fYCLJztq+wTGgnOiyETaEJHt
14BnMCvloMScQDophG75v7hU9W6LaoqjSIQukRUZ7HF6k5E6C0YKRfGWU/TuRygdp8
15++LNdF8H
16=gDWo
17-----END PGP SIGNATURE-----

Timestamp of file with hash 61e0aed6678c57e234d6fe4ca5ad6495ad6a11f3d4a972801fd6dc65a7737a60 -

promag commented at 11:36 pm on June 18, 2020: member

@sipa ping.

sipa commented at 11:58 pm on June 18, 2020: member

utACK 3351c91ed402895dcb4f803a29d2cac70ccfa8b4

in src/script/standard.cpp:276 in 3351c91ed4

299-        return true;
300+        return CScript() << CScript::EncodeOP_N(id.version) << std::vector<unsigned char>(id.program, id.program + id.length);
301     }
302 };
303+
304+const CScriptVisitor g_script_visitor;

MarcoFalke commented at 11:54 am on June 19, 2020:

nit: function-scope globals are better than anon-scope globals, but all this code will go away with C++17, so no big deal

MarcoFalke merged this on Jun 19, 2020

MarcoFalke closed this on Jun 19, 2020

sidhujag referenced this in commit bff30e54f0 on Jul 7, 2020

kittywhiskers referenced this in commit 5b9a4c1acd on Jul 15, 2021

kittywhiskers referenced this in commit 116b3e6b36 on Jul 17, 2021

Fabcien referenced this in commit a112f4d8d3 on Aug 26, 2021

kittywhiskers referenced this in commit 7faa169ba9 on Sep 11, 2021

kittywhiskers referenced this in commit c60491eef1 on Sep 15, 2021

thelazier referenced this in commit ca1f9b5756 on Sep 19, 2021

thelazier referenced this in commit acbbeef88e on Sep 25, 2021

DrahtBot locked this on Feb 15, 2022

refactor: Make CScriptVisitor stateless #18863

Conflicts