new clock implementation and tests #1

stephenctw · 2023-10-02T12:42:36Z

Implement real clock
Achieve 100% function test coverage, excluding machine related ones
Add forge fmt --check and forge test to CI
Update lua node to keep track of block time after fastforwarding
(lua-node) Add two idle players, which will be eliminated by honest player calling eliminateMatchByTimeout

* add `fmt` config * add `fmt` check to CI * formate all contract

stephenctw · 2023-10-03T11:27:49Z

I failed to re-enable Match.t.sol because of this issue.

stephenctw · 2023-11-23T14:46:54Z

@GCdePaula I made some changes based on our discussion awhile ago. But I don't think I've covered everything. Could you submit your comments/reviews so I can double check?

GCdePaula

Amazing work! I've left a few comments.

GCdePaula · 2023-10-10T11:58:05Z

permissionless-arbitration/contracts/src/CanonicalConstants.sol

+    // Dummy
+    Time.Duration constant COMMITMENT_EFFORT = Time.Duration.wrap(0);
+    Time.Duration constant CENSORSHIP_TOLERANCE = Time.Duration.wrap(60 * 5);
+    Time.Duration constant MATCH_EFFORT = Time.Duration.wrap(60 * 2);

    // 4-level tournament
    uint64 constant LEVELS = 3;


I think we need to update this comment.

Sure, what should it be updated to?

GCdePaula · 2023-10-30T20:05:44Z

permissionless-arbitration/lua_node/player/honest_strategy.lua

@@ -241,9 +262,42 @@ function HonestStrategy:_react_tournament(state, tournament)
        end
    end

+    for _, match in ipairs(tournament.matches) do


One issue is that this would only run at the tournaments the player is on.

I propose a different approach. Let's separate the two logics. The player only plays the tournaments and matches. Separate from this, we have a garbage collector, which will scan all the tournaments recursively, calling timeout on all matches.

GCdePaula · 2023-12-11T20:32:13Z

permissionless-arbitration/contracts/src/Clock.sol

-        }
+        Time.Duration _allowance =
+            _timeLeft.add(ArbitrationConstants.MATCH_EFFORT);
+        // TODO: do we need this cap check?


I think we need it.

GCdePaula · 2023-12-11T22:41:33Z

permissionless-arbitration/contracts/src/tournament/abstracts/LeafTournament.sol

            );

-            _clockOne.addValidatorEffort(Time.ZERO_DURATION);
+            _clockOne.deduct(_clockTwo.timeSinceTimeout());


I'm not sure about this. I believe both clocks are running on step in the leaf match. I think if we deduct, we'll be subtracting it twice.

I think we need to pause it too.

I see, so a simple setPaused() should do it here.

GCdePaula · 2023-12-11T22:45:38Z

permissionless-arbitration/contracts/src/tournament/abstracts/NonRootTournament.sol

+    /// - if the tournament is finished
+    /// - the contested parent commitment
+    /// - the dangling commitment
+    function innerTournamentWinner()


Let's add a mental TODO on how to deal with tournaments where nobody won.

Besides no one joining, all claims can also time out!

I think we need to be able to communicate to the caller:

Tournament not finished;

Tournament finished, no one won (parent can delete parent match)

Tournament finished, here's the winner.

Let's open an issue for this. We need to change this function, and also the caller of this function.

GCdePaula · 2023-12-11T22:47:39Z

permissionless-arbitration/contracts/src/tournament/abstracts/Tournament.sol

-        } else {
-            maximumEnforceableDelay = Time.currentTime().add(_allowance);
-        }
+        // TODO: do we need this cap check?


I think we also need this check :/

Maybe the caller should make sure of that instead? I'm not sure.

GCdePaula · 2023-12-11T22:52:22Z

permissionless-arbitration/lua_node/blockchain/reader.lua

-            return true
-        elseif m1.block_number > m2.block_number then
-            return false
+function CommitmentClock:display(block_time)


Check the __tostring metamethod!

GCdePaula · 2023-12-11T22:54:39Z

permissionless-arbitration/lua_node/blockchain/reader.lua

+    assert(handle)
+
+    local ret
+    local str = handle:read()


Could you read everything instead with *a? Might simplify this loop.

GCdePaula

Great work :)

GCdePaula · 2023-12-20T11:05:32Z

permissionless-arbitration/contracts/src/tournament/abstracts/NonRootTournament.sol

+    /// - if the tournament is finished
+    /// - the contested parent commitment
+    /// - the dangling commitment
+    function innerTournamentWinner()


Besides no one joining, all claims can also time out!

GCdePaula · 2023-12-20T11:07:21Z

permissionless-arbitration/contracts/src/tournament/abstracts/NonRootTournament.sol

+    /// - if the tournament is finished
+    /// - the contested parent commitment
+    /// - the dangling commitment
+    function innerTournamentWinner()


I think we need to be able to communicate to the caller:

Tournament not finished;

Tournament finished, no one won (parent can delete parent match)

Tournament finished, here's the winner.

GCdePaula · 2023-12-20T11:07:48Z

permissionless-arbitration/contracts/src/tournament/abstracts/NonRootTournament.sol

+    /// - if the tournament is finished
+    /// - the contested parent commitment
+    /// - the dangling commitment
+    function innerTournamentWinner()


Let's open an issue for this. We need to change this function, and also the caller of this function.

stephenctw added 3 commits September 21, 2023 18:51

feat(onchain): new clock design

48d885b

chore: configure solidity formatter

4756108

* add `fmt` config * add `fmt` check to CI * formate all contract

feat: update tests to new clock design

b1af00d

stephenctw force-pushed the feature/clock branch from bbf151a to a05c759 Compare October 2, 2023 12:49

test: increase coverage (100% functions)

6831e87

stephenctw requested a review from GCdePaula October 3, 2023 11:13

stephenctw self-assigned this Oct 3, 2023

feat(lua-node): adapt blockchain fastforward

3078b14

stephenctw force-pushed the feature/clock branch from a05c759 to 3078b14 Compare October 3, 2023 11:20

stephenctw added the enhancement New feature or request label Oct 3, 2023

stephenctw added 3 commits October 4, 2023 20:21

feat(contracts): add eliminateMatchByTimeout

c3f9855

feat(lua-node): add idle strategy, add idle players and pass luacheck

691b70e

chore(lua-node): improve code quality

78f9b87

GCdePaula reviewed Dec 11, 2023

View reviewed changes

stephenctw requested a review from GCdePaula December 15, 2023 12:05

stephenctw added 9 commits December 20, 2023 17:48

chore(lua-node): improve clock class

3e48a44

chore(contract): resolve PR review

33a194e

chore(lua-node): add block_time to CommitmentClock

01e5457

fix(lua-node): isolate garbage collection from HonestStrategy

9138f8f

chore: simplify _get_block parser

2d8f1bc

chore(contract): apply forge fmt

379d65f

fix: fix forge test

8f6ae4f

fix: fix machine-emulator version to 0.15.2

f161aa0

chore(lua-node): luacheck warnings

6bfea30

stephenctw force-pushed the feature/clock branch from 010fc1a to 6bfea30 Compare December 20, 2023 10:48

chore(contract): fix comment

6b46f63

GCdePaula approved these changes Dec 21, 2023

View reviewed changes

stephenctw merged commit 8819b03 into main Dec 22, 2023
1 check passed

GCdePaula deleted the feature/clock branch March 23, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new clock implementation and tests #1

new clock implementation and tests #1

stephenctw commented Oct 2, 2023 •

edited

Loading

stephenctw commented Oct 3, 2023

stephenctw commented Nov 23, 2023

GCdePaula left a comment

GCdePaula Oct 10, 2023

stephenctw Dec 12, 2023 •

edited

Loading

GCdePaula Oct 30, 2023

GCdePaula Dec 11, 2023

GCdePaula Dec 11, 2023

stephenctw Dec 12, 2023

GCdePaula Dec 11, 2023

GCdePaula Dec 20, 2023

GCdePaula Dec 20, 2023

GCdePaula Dec 20, 2023

stephenctw Dec 22, 2023

GCdePaula Dec 11, 2023

GCdePaula Dec 11, 2023

GCdePaula Dec 11, 2023

GCdePaula Dec 11, 2023

GCdePaula left a comment

GCdePaula Dec 20, 2023

GCdePaula Dec 20, 2023

GCdePaula Dec 20, 2023

new clock implementation and tests #1

new clock implementation and tests #1

Conversation

stephenctw commented Oct 2, 2023 • edited Loading

stephenctw commented Oct 3, 2023

stephenctw commented Nov 23, 2023

GCdePaula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephenctw Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GCdePaula left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephenctw commented Oct 2, 2023 •

edited

Loading

stephenctw Dec 12, 2023 •

edited

Loading