Fix watch - [merged] #354

Closed kwannoel closed 3 years ago

In GitLab by @kwanzknoel on Jun 9, 2021, 23:46

Merges noel/fix-watch -> master

Fixes #195, #202

cc @plotnick

Requires: https://github.com/fare/gerbil-ethereum/pull/42, since there are breaking changes to watch-contract's interface.

In GitLab by @kwanzknoel on Jun 9, 2021, 23:46

assigned to @kwanzknoel

In GitLab by @kwanzknoel on Jun 9, 2021, 23:47

changed the description

In GitLab by @kwanzknoel on Jun 9, 2021, 23:47

requested review from @plotnick

In GitLab by @kwanzknoel on Jun 10, 2021, 03:02

changed title from Draft: Fix {-glow-path in pass command-} to Draft: Fix {+watch+}

In GitLab by @kwanzknoel on Jun 10, 2021, 04:36

added 1 commit

3c4e7621 - Interim commit

Compare with previous version

In GitLab by @kwanzknoel on Jun 10, 2021, 04:38

changed the description

In GitLab by @kwanzknoel on Jun 11, 2021, 05:01

added 1 commit

0dbe3408 - Handle multiple logs

Compare with previous version

In GitLab by @kwanzknoel on Jun 14, 2021, 15:14

Following error occurs:

Assertion Error

With this trace:

*** ERROR IN clan/exception#call-with-logged-exceptions__% -- Assertion failed
0  clan/exception#call-with-logged-exceptions__% 
1  ##dynamic-env-bind                   
2  mukn/glow/runtime/participant-runtime#interpret-current-code-block 
3  mukn/glow/runtime/participant-runtime#interpret-current-code-block 
4  mukn/glow/runtime/participant-runtime#run-passive-code-block/contract 
5  mukn/glow/runtime/participant-runtime#execute-1 
6  mukn/glow/runtime/participant-runtime#execute 
7  ##dynamic-env-bind                   
8  ##dynamic-env-bind                   
9  ##dynamic-env-bind                   
10 mukn/glow/runtime/reify-contract-parameters#run 
11 ##dynamic-env-bind                   
12 ##dynamic-wind                       
13 mukn/glow/cli/interaction#start-interaction__% 
14 ##dynamic-env-bind                   
15 ##call-with-values                   
16 clan/exit#call-print-exit            
17 ##dynamic-env-bind                   
18 ##start-main                         
19 ##kernel-handlers

In GitLab by @kwanzknoel on Jun 14, 2021, 22:49

It is likely due to changes in glow/runtime/watch logic. Changes to gerbil-ethereum/watch have been tested.

In GitLab by @kwanzknoel on Jun 14, 2021, 23:01

added 2 commits

83faa01c - Use watch-batch interface
2535d21a - Fix initialization for first-unprocessed-block

Compare with previous version

In GitLab by @kwanzknoel on Jun 15, 2021, 00:12

It works after swapping out the following line:

      ;; (def from-block first-unprocessed-block)
      (def from-block
        (if (.@ self timer-start)
        (+ (.@ self timer-start) 1)
        (.@ contract-config creation-block)))

In GitLab by @kwanzknoel on Jun 15, 2021, 00:13

Commented on runtime/participant-runtime.ss line 217

Tests pass after swapping out this line to:

      (def from-block
        (if (.@ self timer-start)
        (+ (.@ self timer-start) 1)
        (.@ contract-config creation-block)))

In GitLab by @kwanzknoel on Jun 15, 2021, 00:27

Commented on runtime/participant-runtime.ss line 217

timer-start is always updated to the first block number which comes in after watching.

unprocessed-block is always updated to the last block number in a watch-batch.

However, all the other unprocessed-blocks are kept in unprocessed-events slot in Runtime.

In GitLab by @kwanzknoel on Jun 15, 2021, 00:29

Commented on runtime/participant-runtime.ss line 217

Another variable is initialization: timer-start is initialized to maxInitialBlock, unprocessed-block is initialized to (.@ contract-config creation-block).

UPDATE: If we initialize both to be the same, the first watch passes, and the second fails.

In GitLab by @kwanzknoel on Jun 15, 2021, 00:32

Commented on runtime/participant-runtime.ss line 217

Hypothesis 1: unprocessed-block causes overlap into other participants' watch blocks. The log-data we obtain is from previous participants.

We should observe same logdata if this is the case.

UPDATE: The contract fails on second watch, with duplicate logs (need to synchronize with runtime frame, timer-start is synchronized which is why there is no problem).

In GitLab by @kwanzknoel on Jun 15, 2021, 22:03

Commented on runtime/participant-runtime.ss line 217

This is not enough however, we need to also synchronize which events we have seen, otherwise we might skip some events in previous blocks.

In GitLab by @kwanzknoel on Jun 15, 2021, 22:03

Commented on runtime/participant-runtime.ss line 217

See comments on watch from fare.

In GitLab by @kwanzknoel on Jun 16, 2021, 18:29

changed the description

In GitLab by @kwanzknoel on Jun 16, 2021, 21:03

Commented on runtime/participant-runtime.ss line 217

changed this line in version 5 of the diff

In GitLab by @kwanzknoel on Jun 16, 2021, 21:03

added 10 commits

7d5baa08 - Add failing tests for buy_sig
80ab7722 - Comments on fixing watch
ca6de769 - Fix glow-path in pass command
913e5c2d - Track first unprocessed block in Runtime
4284189b - Interim commit
24f4a8b3 - Handle multiple logs
a8e38b01 - Use watch-batch interface
514986cd - Fix initialization for first-unprocessed-block
2806c70d - Add debug stmts, remove and rewrite before merge
0348041d - Pass testcases for watch

Compare with previous version

In GitLab by @kwanzknoel on Jun 16, 2021, 21:04

Commented on runtime/participant-runtime.ss line 99

revert, only leave typo changes.

In GitLab by @kwanzknoel on Jun 16, 2021, 21:05

Commented on runtime/participant-runtime.ss line 105

remove comments. These are self-explanatory. Further detail can be provided in the runtime documentation #200

In GitLab by @kwanzknoel on Jun 16, 2021, 21:05

Commented on runtime/participant-runtime.ss line 131

revert, unprocessed-events currently not used.

In GitLab by @kwanzknoel on Jun 16, 2021, 21:06

Commented on runtime/participant-runtime.ss line 138

We need to update this whenever we update first-unprocessed-block.

In GitLab by @kwanzknoel on Jun 16, 2021, 21:07

Commented on runtime/participant-runtime.ss line 205

strip all DBG statements

In GitLab by @kwanzknoel on Jun 16, 2021, 21:08

Commented on runtime/participant-runtime.ss line 215

remove