Hide DAG implementation from protocol specification#28
Merged
Conversation
This allows to replace the DAG implementation with something else. I do that because at some point I want to try a persistent implementation that allows to fork simulations. Time-travelling might also be useful at some point. WIP. I have no time to fix all the protocols now.
Some protocols seem to work. I still have problems with the tailstorms. It seems that some prior uses of interate_ancestors were relying on the partial order. The new confirmed_votes function does not return partially sorted stuff. backlog: - blockDAG type key is not needed. We can compare blocks directly imo - tailstorm[ll] acc_votes should use set instead of list - type state = block should be something like type state = Preferred block - there was no good reason to rename max|min_pow to max|min_hash. Undo! - Heuristic quorum uses hashtbl where is could use set - confirming|ed_votes should assert that argument is block. Otherwise abuse - Is Ssz_tools.State8 used anywhere? - Use blockdag.neq where applicable. I replaced it with eq in some places - I removed the distinction between localview and globalview. maybe unwise. revert?
Backlog: - blockDAG type key is not needed. We can compare blocks directly imo - type state = block should be something like type state = Preferred block - there was no good reason to rename max|min_pow to max|min_hash. Undo! - Heuristic quorum uses hashtbl where it could use set - Is Ssz_tools.State8 used anywhere? - I removed the distinction between localview and globalview. maybe unwise. revert? - In simulator we use 'prot_data type variable. this can now be 'a - Visually verify that tailstorm[ll] reward schemes are correct
Previously, we accumulated votes, then filtered them. This fix avoids returning unconnected sets of votes if the agent decides to ignore defender votes after having extended upon such a vote. Still WIP, but protocol tests seem to work now. Backlog: - blockDAG type key is not needed. We can compare blocks directly imo - type state = block should be something like type state = Preferred block - there was no good reason to rename max|min_pow to max|min_hash. Undo! - Heuristic quorum uses hashtbl where it could use set - Is Ssz_tools.State8 used anywhere? - I removed the distinction between localview and globalview. maybe unwise. revert? - In simulator we use 'prot_data type variable. this can now be 'a - Visually verify that tailstorm[ll] reward schemes are correct
Backlog: - blockDAG type key is not needed. We can compare blocks directly imo - type state = block should be something like type state = Preferred block - there was no good reason to rename max|min_pow to max|min_hash. Undo! - Heuristic quorum uses hashtbl where it could use set - Is Ssz_tools.State8 used anywhere? - I removed the distinction between localview and globalview. maybe unwise. revert? - In simulator we use 'prot_data type variable. this can now be 'a - Visually verify that tailstorm[ll] reward schemes are correct
Dag now has a Block sub-module which can be passed to Dagtools.Make and Set.Make
pkel
commented
Dec 2, 2022
pkel
force-pushed
the
hide-dag-from-protocols
branch
from
a8c3a9b to
fb5299c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently, details of the DAG implementation leak into the protocol specification.
Implies that protocol specs cannot be used with different DAG.
I want to play around with a persistent/immutable DAG implementation.
This can may enable forkable simulations in the future.
In this PR I do the first step in this direction. I hide the DAG implementation from the protocol specification API.
The better part of the work is done. But a few todos accumulated:
type keyis not needed. We can compare blocks directly imo.type state = blockshould be something liketype state = Preferred block. This will improve error messages. Skipped, not necessary, increases diff, orthogonal.max|min_powtomax|min_hashin the spec API. Undo!tailstorm[ll]heuristic quorum uses hashtbl where it could use a Set.Ssz_tools.State8used anywhere? If not, drop!localviewandglobalview. May be unwise. Revert? No. I like the new interface.'prot_datatype variable. This can now be'a.tailstorm[ll]reward schemes are correct.ethereum_test.ml. Have no simpler implementation. Thus ignore.dag.mlimplementation and newdagtools.ml