-
Notifications
You must be signed in to change notification settings - Fork 12
Conversation
stephenctw
commented
Sep 2, 2023
•
edited
Loading
edited
- Separate players in different Lua VMs
- Fast-forward blockchain
- Separate fetching state and logic
- Divide into strategy and machine
* Rename `utils.log` to `utils.helper` * Fastforward blockchain when all players idle
c86a81e
to
6f5c759
Compare
@GCdePaula I'm not sure about the last item: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's looking great!
-- if all players are idle for 10 consecutive iterations, advance blockchain | ||
if all_idle == 5 then | ||
print("all players idle, fastforward blockchain for 30 seconds...") | ||
client:advance_time(30) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put this 30
in a variable/constant at the top of the file? Like local FF_TIME = 30
, this way we can easily spot and change this value. Or maybe to some of the constants
table?
local Player = {} | ||
Player.__index = Player | ||
|
||
function Player:new(root_tournament_address, player_index, commitment_builder, machine_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this type should be called State
. We can make it leaner I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need the player_index
, I think it's just used for sending tx. Can you verify? Maybe it makes sense to split the client into reader/sender.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The machine_path
is also not needed, since we want this State
to be used by everyone (honest/dishonest), that may not be using a machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even commitment_builder
I think we need to review, since it seems the State
module is responsible for building commitments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we first need to define a pair of types: tournament and match.
Tournament can be constructed from its address and optional parent match. It has:
- address;
- optional pointer to its parent match;
- level;
- list of commitments made at that tournament;
- list of matches.
Match can be constructed from its tournament. It has:
- tournament it's in;
- commitment one and two;
- current left, right, height, and running leaf;
- leaf_cycle and base_big_cycle;
- optional child tournament.
With that, I believe we can build the whole "tree" of tournament/match. It mirrors the onchain structure. Just by passing the address of the root tournament, the whole structure is built. I think the main difference is that the current code only fetches the matches/tournaments the player is in. The suggestion fetches everything. We'll need that for the simplified clock.
We also remove dependency on machine and commitment builder.
The next part is the strategy, which may need to "search" its commitments on this tournament/match structure. So it may be a good idea to add search/find methods. Like, tournament:find_match_with_commitment(c)
or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I'll work on this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll need a new event when someone joins the tournament in order to track all commitments.
event commitmentJoined(Tree.Node root);
@GCdePaula what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed! Although I think we should make it event commitmentJoined(Tree.Node left, Tree.Node right);
. It's possible we may even remove the matchCreated
event, since we can derive that from the commitmentJoined
event (every pair of consecutive matchCreated
implies a commitmentJoined
). But let's leave this second step for later, unless you wanna do it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created an issue to track this topic.
#242
end | ||
end | ||
|
||
local function _react_honestly(player) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can turn this whole thing into this HonestStrategy
. So all strategies implement the method react
, but the honest one does it honestly. Maybe we can create a nice type for it.
* Split `Client` into `Reader` and `Sender` * Add variables to control behaviors in entrypoint * Add `HonestStrategy` concrete type * Move `pcall` into `Sender:_send_tx` * `State` tracks all tournaments/commitments/matches * Optimize `tournament` and `match` structure * Optimize `player.idle` logic * Add `commitmentJoined` event
3b84f8c
to
fd89843
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work ✨✨✨