-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Smart contracts: Registry + Challenger #22
Conversation
v0.2.1
… into 04-29-chore_contracts_dir
v1.8.1
a1bac2a
to
6e50fc7
Compare
6e50fc7
to
485c8d0
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.
Some details but overall great job!
bolt-contracts/script/Counter.s.sol
Outdated
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.
We can remove this file
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 did it in the upstream PR #35
Will leave it here for now
/// @notice A challenge requires a bond to be transferred to this contract to avoid spamming. | ||
/// @param _basedProposer The address of the proposer to challenge | ||
/// @param _signedCommitment The signed commitment that the proposer is getting challenged for | ||
function challengeProposer(address _basedProposer, SignedCommitment calldata _signedCommitment) public payable { |
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.
Should we check if _basedProposer
is in the registry or we leave it as it is?
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 genelized index is the index of the merkle tree generated by the merkleization | ||
// process of a SSZ list of transactions. Since this list is dynamic and can be of maximum | ||
// length of 2^20 = 1_048_576, the merkleization process fills the tree with empty hashes, | ||
// therefore this number is an offset from where transactions hash tree root starts. | ||
// To read more, check out https://github.com/ethereum/consensus-specs/blob/dev/ssz/simple-serialize.md#merkleization | ||
uint256 generalizedIndex = 1_048_576 + _transactionIndex; |
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 generalized index actually starts at 2^21 = 2_097_152
, because we have to take into account one level of the tree that specifies the maximum length of the list
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.
Good catch ser!
This PR includes basic tests for both registry and challenger, but more comprehensive tests will be added in further PRs.
In particular, the challenger logic is security critical so that will need to undergo a lot more effort.