Split up EvalScript into EvalScriptState::Start and ::Step, so single-stepping can be done #3901

1. 
   luke-jr commented at 4:09 pm on March 19, 2014: member

   The idea being that once scripting is its own shared library, one can have a debugger than single-steps through a Script.
2. in src/script.cpp: in a7250c2c4e outdated

   363+    fEof = !(pc < pend);
   364+    if (fEof)
   365+    {
   366+        if (!vfExec.empty())
   367+            return false;
   368+        return true;
   

   ---

   ---

   

   unknown commented at 4:47 pm on March 19, 2014:

   I’m assuming vfExec.empty() returns boolean, so could you simplify with return vfExec.empty()?
3. 
   petertodd commented at 7:46 pm on March 22, 2014: contributor

   ACK on the idea of making some debugging tools, but a very, very strong NACK on actually changing the consensus-critical code that gets executed by default. It’s just unnecessarily risky for little gain when you can just as easily make a copy of the code. Sure, the latter is a annoying to maintain, but I’ll take “annoying to maintain” over “accidentally cause a fork” any day, particularly since the code being copied rarely ever changes.
4. 
   sipa commented at 9:09 pm on March 24, 2014: member

   I fully agree that changing consensus-critical code is dangerous, but not that copying is the right solution.

   The purpose of the reference client code shouldn’t just be being “right” (even though that has very high priority). It should also be clear, transparent and reusable. If it’s not reusable, people will either reimplement it (and make mistakes), or copy & modify & make mistakes.

   I’m very much in favor of creating a low-dependency (ideally: none) library that separates out the script evaluation functionality (and later on, one that implements the validation code without actual disk storage, peer communication, … ).

   Design ACK here, but I haven’t reviewed the code changes yet.

5. 
   petertodd commented at 6:57 am on March 25, 2014: contributor

   @sipa Agreed on the library, but that doesn’t exist yet, and this is a very, very high-risk case. If anything, maybe what that is saying is that we should just hold off on this code until we’ve got that library and can actually make use of it.
6. 
   sipa commented at 11:26 am on June 1, 2014: member

   Rebase please?
7. Split up EvalScript into EvalScriptState::Start and ::Step, so single-stepping can be done 704672f36d
8. 
   luke-jr commented at 2:50 pm on June 26, 2014: member

   Rebased, with @drak ’s suggestion.
9. 
   BitcoinPullTester commented at 5:14 pm on June 26, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p3901_704672f36da19535aa81d0dbf950c696809ba0dd/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
10. 
    jgarzik commented at 5:31 pm on July 29, 2014: contributor

    I really like the low diff size of this change. It helps reviewing and decreases risk IMO.
11. laanwj added the label TX scripts on Jul 31, 2014
12. 
    sipa commented at 6:51 pm on September 27, 2014: member

    Closing due to inactivity. I’m personally still in favor of this, but should happen after the ongoing script refactorings.
13. sipa closed this on Sep 27, 2014

    ---

14. DrahtBot locked this on Sep 8, 2021

Contributors
luke-jr petertodd sipa BitcoinPullTester jgarzik

Labels
Validation