forked from cosmos/cosmos-sdk
-
Notifications
You must be signed in to change notification settings - Fork 34
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
feat: Adds validator qgb checks #137
Merged
rach-id
merged 22 commits into
celestiaorg:rachid/orchestrator-address
from
rach-id:adds_validator_qgb_checks
May 9, 2022
Merged
Changes from all commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
0e9a2db
first pass on tests fixes
rach-id 49fadc4
fixes the rest of unit tests
rach-id c3fe6aa
remove unnecessary comments
rach-id 168a6fe
uses default eth address when starting sim network
rach-id af04769
cosmetics
rach-id bbf69d4
comments failing test
rach-id a910030
comments failing test
rach-id 756f041
adds orchestrator/ethereum address checks for validators when creatin…
rach-id e70bbd8
uses correct error codes for eth/orch address errors
rach-id f53f9d6
uncomments test and fixes it from commit 434b308ce4dc968070c47952d948…
rach-id edf6b87
Merge branch 'fix_tests_after_qgb_changes' into adds_validator_qgb_ch…
rach-id 8568eac
adds zero eth address check when creating/updating validator
rach-id b6fbf3a
attempts to fix duplicate eth address in sim network
rach-id a10f734
Merge branch 'rachid/orchestrator-address' into adds_validator_qgb_ch…
rach-id 3dbbe6c
revert squashed changes
rach-id a547c1b
uses unwrapped context for msg_server orch/eth validation
rach-id 03e8b0c
increase DefaultGenTxGas to accomodate new qgb validator changes
rach-id fd79550
scaffolds an ethereum address from orchestrator address in staking si…
rach-id a799b09
fix eth address creation from orch address in operations
rach-id fa29549
increase DefaultGenTxGas
rach-id 308853d
increase DefaultGenTxGas
rach-id 176d296
simpler way of creating an eth address from orch address in operations
rach-id File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
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.
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.
Are you sure you want to error when a validator exists if you're trying to edit it?
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 don't want to have duplicate orchestrator or Ethereum address. If some other validatore has that address, we shouldnt continue. No?
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.
For inserting a new validator, yes. But editing a validator (not sure what that even means?) requires the validator to exist prior to being edited, no?
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 look first if the validator exists by its address. Then, we start checking if the new orchestrator address has been given to some other validator.
So, that one is looking for a validator by orchestrator address. If any other validator has that orchestrator address, we stop.
Or I am missing 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.
I think this makes sense, we want to throw an error if someone is trying to update their vaildator with an invalid orchestrator address. If we don't, then we won't be able to verify their attestations, and while it wouldn't be protocol breaking, I think it would be a terrible UX, as they'd have to debug why they can't submit signatures.
we're also checking that the validator exists on line 143