Initial Governance contract integration #147
Merged
Conversation
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
Member
Author
|
@chenquanyu, @txhsl welcome to review, but please keep in mind that it's a very early version of integration. @nicolegys, this PR is not ready for testing yet, I'll notify you as soon as it's ready. |
There's a chance that newly-accepted block will be added by worker using the corresponding sealing task saved in the worker state. If so, then BlockQueue don't have any way to track the end of block addition process. Since postBlock contains NextConsensus computations, it must be called only after successfull block persist so that the proper state is available in the chain. And thus, call postBlock only when chain notification arrives. A part of #14. An example of problem logs that this commit is aimed to fix: ``` 2024-03-11T19:52:22.324+0300 DEBUG [email protected]/dbft.go:214 received message {"type": "Commit", "from": 5, "height": 1, "view": 1, "my_height": 1, "my_view": 1} 2024-03-11T19:52:22.324+0300 DEBUG [email protected]/dbft.go:214 received message {"type": "Commit", "from": 6, "height": 1, "view": 1, "my_height": 1, "my_view": 1} 2024-03-11T19:52:22.324+0300 INFO [email protected]/dbft.go:511 received Commit {"validator": 6} 2024-03-11T19:52:22.325+0300 INFO [email protected]/check.go:66 approving block {"height": 1, "hash": "8633939a9fa52e0fc260b8dc335f21bf11c5519ea150683d434e4986f597a294", "tx_count": 0, "merkle": "eeccdc5021307b4b06231fcd6c7b7a9c632863d647459bb4124e9c95f26e50f5", "prev": "9ee34b625f2e9aac3d0b7aaaea40db845906d6d24c89e3e4f53af8d63074eb83"} INFO [03-11|19:52:22.325] calculating validators height=1 CRIT [03-11|19:52:22.325] failed to compute next block validators err="header not found" ``` Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva
force-pushed
the
governance-integration
branch
from
f6389ae to
46cbff6
Compare
Merged
Member
Author
|
@roman-khimov, first version is ready for review. The contract version should be updated (currently the very old one is used to test 7-nodes privnet), but the idea of the PR won't be changed if no issues found. |
chenquanyu
reviewed
Mar 15, 2024
chenquanyu
reviewed
Mar 15, 2024
core/systemcontracts/contracts.go
Outdated
| // TODO: need to finalize genesis contract hashes in https://github.com/bane-labs/go-ethereum/pull/146 and https://github.com/bane-labs/go-ethereum/pull/7 | ||
| GovernanceHash = "0x1212000000000000000000000000000000000001" | ||
| // GovernanceABI is a compiled ABI of Governance contract. | ||
| GovernanceABI = `[{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"candidate","type":"address"},{"indexed":false,"internalType":"uint256","name":"reward","type":"uint256"}],"name":"CandidateClaim","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"candidate","type":"address"}],"name":"Exit","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"candidate","type":"address"}],"name":"Register","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"voter","type":"address"},{"indexed":false,"internalType":"address","name":"from","type":"address"},{"indexed":false,"internalType":"uint256","name":"amount","type":"uint256"}],"name":"RevokeVote","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"voter","type":"address"},{"indexed":false,"internalType":"address","name":"to","type":"address"},{"indexed":false,"internalType":"uint256","name":"amount","type":"uint256"}],"name":"Vote","type":"event"},{"anonymous":false,"inputs":[{"indexed":false,"internalType":"address","name":"voter","type":"address"},{"indexed":false,"internalType":"uint256","name":"votes","type":"uint256"},{"indexed":false,"internalType":"uint256","name":"reward","type":"uint256"}],"name":"VoterWithdraw","type":"event"},{"inputs":[],"name":"EPOCH_DURATION","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"MIN_TOTAL_VOTE","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"MIN_VOTE_AMOUNT","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"REGISTER_FEE","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"candidateBalanceOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"candidateClaim","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"candidateList","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"claimRegisterFee","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"claimStartEpochOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"epochCount","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"epochRewards","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"exitCandidate","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"exitEpochOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"uint256","name":"epoch","type":"uint256"}],"name":"getConsensus","outputs":[{"internalType":"address[7]","name":"","type":"address[7]"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"getCurrentConsensus","outputs":[{"internalType":"address[7]","name":"","type":"address[7]"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"getNominalCurrentEpoch","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"getRealCurrentEpoch","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"candidate","type":"address"},{"internalType":"uint256","name":"epoch","type":"uint256"}],"name":"getReceivedVotesByEpoch","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"voter","type":"address"},{"internalType":"uint256","name":"epoch","type":"uint256"}],"name":"getVotedByEpoch","outputs":[{"internalType":"address","name":"","type":"address"},{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"govReward","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"lastEpochTime","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"},{"internalType":"uint256","name":"","type":"uint256"}],"name":"receivedVotes","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"uint256","name":"shareRate","type":"uint256"}],"name":"registerCandidate","outputs":[],"stateMutability":"payable","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"registerEpochOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"revokeVote","outputs":[],"stateMutability":"nonpayable","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"}],"name":"shareRateOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"totalVotes","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"},{"internalType":"uint256","name":"","type":"uint256"}],"name":"unclaimedEpochsOf","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"candidateTo","type":"address"}],"name":"vote","outputs":[],"stateMutability":"payable","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"},{"internalType":"uint256","name":"","type":"uint256"}],"name":"votedAmount","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"uint256","name":"","type":"uint256"}],"name":"votedCandidates","outputs":[{"internalType":"uint256","name":"","type":"uint256"}],"stateMutability":"view","type":"function"},{"inputs":[{"internalType":"address","name":"","type":"address"},{"internalType":"uint256","name":"","type":"uint256"}],"name":"votedTo","outputs":[{"internalType":"address","name":"","type":"address"}],"stateMutability":"view","type":"function"},{"inputs":[],"name":"voterWithdraw","outputs":[],"stateMutability":"nonpayable","type":"function"},{"stateMutability":"payable","type":"receive"}]` |
Collaborator
There was a problem hiding this comment.
May be we only need the interface abi here.
Member
Author
There was a problem hiding this comment.
Yes, it will be updated once we have Governance finalized.
AnnaShaleva
force-pushed
the
governance-integration
branch
3 times, most recently
from
c1ff0e9 to
65f7bd7
Compare
chenquanyu
reviewed
Mar 23, 2024
AnnaShaleva
force-pushed
the
governance-integration
branch
2 times, most recently
from
b34a042 to
e6ae336
Compare
Member
Author
|
@roman-khimov, ready for review & merge, @nicolegys is performing 7-nodes privnet testing. Should we wait for test results before the merge? |
Contributor
|
We need to test voting that changes the set of validators, btw. |
Collaborator
|
CNs failed when trying to change epoch. |
Contributor
Have you tried to register at least 7 different addresses as candidates before epoch change? The related contract method is https://github.com/bane-labs/go-ethereum/blob/governance/contracts/solidity/Governance.sol#L119. |
Member
Author
Node expects the contract to properly handle
So I'd expect the contract to handle the case when not enough candidates are registered. In this case StandBy validators should be returned. Because we will have such cases in the real network. |
AnnaShaleva
force-pushed
the
governance-integration
branch
from
0aed4be to
7d7f4e6
Compare
A part of #14. Refer to Governance contract to get next validators addresses. Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva
force-pushed
the
governance-integration
branch
from
7d7f4e6 to
5d998b3
Compare
No functional changes. Signed-off-by: Anna Shaleva <[email protected]>
For better further debugging experience. Signed-off-by: Anna Shaleva <[email protected]>
Genesis-based validators retrieval may be done using the same approach as for further blocks. This change allows us to remove StandByValidators from dBFT configuration in future. Signed-off-by: Anna Shaleva <[email protected]>
We need an access to block state at the PrepareRequest construction level to be able to calculate NextConsensus for the current proposal. State for the 0-th view can be fetched from miner, but I'd avoid adding one more miner-dependent state field to the dBFT engine, especially given the fact that one day we'll remove state calculations from miner. Signed-off-by: Anna Shaleva <[email protected]>
Calculate NextConsensus for accepting block using the state of accepting block. The state is retrieved either during PrepareRequest construction (by Primary) or during PrepareRequest verification (by backups). Thanks to this change no additional APIs are required from Governance contract, the only `getCurrentConsensus` API is necessary. A part of #14. Signed-off-by: Anna Shaleva <[email protected]>
If validators are requested by block height then the block state is already in chain and accepted, and thus, the set of validators computed based on this state won't change. Cache validators for three most recent heights which is enough for effective network payloads processing (we may have stale payloads travelling through the network) and dBFT fuctioning (it always needs the latest set of validators for block signing). Signed-off-by: Anna Shaleva <[email protected]>
This method returns exactly validators of the latest finalized epoch from the specified block/state PoW. Signed-off-by: Anna Shaleva <[email protected]>
Finalize is called by state processor during PrepareRequest state verification, and thus, don't need to call it one more time manually in WithNewPrepareRequest callback. Signed-off-by: Anna Shaleva <[email protected]>
No errors expected during decoding since these are trusted data. Signed-off-by: Anna Shaleva <[email protected]>
Use the same approach as for Beacon block root processing via system contract. Signed-off-by: Anna Shaleva <[email protected]>
Signed-off-by: Anna Shaleva <[email protected]>
AnnaShaleva
force-pushed
the
governance-integration
branch
from
5d998b3 to
a8bf2ee
Compare
roman-khimov
approved these changes
Mar 27, 2024
AnnaShaleva
added a commit
that referenced
this pull request
Jun 14, 2024
Exactly like it's done with BeaconBlockRoot. Should be a part of #147. Signed-off-by: Anna Shaleva <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Here's a draft that I have right now, several questions need to be decided, they are marked as TODOs in the code. Close #14.
The good news is that the code is able to call Governance API and get the current set of validators. The bad news is that sometimes contract call doesn't work as expected, the following error occurs on attempt to unpack the result, I'm investigating it.UPD: the described problem is solved, now Governance calls work as expected. Now I'm working on the rest of less critical TODOs:
Automate contract build and genesis allocations pre-filling.Will be done in Genesis contracts related automation #149.