Skip to content
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

QGB add validator edit test cases #768

Closed
Tracked by #301
rach-id opened this issue Sep 22, 2022 · 2 comments · Fixed by #1048
Closed
Tracked by #301

QGB add validator edit test cases #768

rach-id opened this issue Sep 22, 2022 · 2 comments · Fixed by #1048
Assignees
Labels
good first issue Good for newcomers testing items that are strictly related to adding or extending test coverage

Comments

@rach-id
Copy link
Member

rach-id commented Sep 22, 2022

If a validator edits their orchestrator/ethereum address, the state machine must react to that.
Currently:

  • if they change their orchestrator address, then it's alright, nothing will change
  • If they change their Ethereum address, they will be treated like a new validator and a new valset will be emitted and submitted to the target EVM chain.

We need to have tests for this.

@rach-id rach-id added good first issue Good for newcomers C: QGB testing items that are strictly related to adding or extending test coverage labels Sep 22, 2022
@rach-id rach-id moved this to TODO in Celestia Node Sep 22, 2022
@evan-forbes
Copy link
Member

evan-forbes commented Nov 8, 2022

note from sync: we need to think about this more, specifically we need to know for certain that the validator set changes in the endblocker if this occurs and how that relates slashing

we might also to move this to our fork of the cosmos-sdk

@rach-id rach-id self-assigned this Nov 21, 2022
@rach-id
Copy link
Member Author

rach-id commented Nov 21, 2022

how that relates slashing

I guess in the case of editing the validator orch address, we will still be able to identify them as a malicious validator. Because, from the QGB perspective, we identify validators by their EVM addresses.

However, if they edit their EVM address, it becomes harder to catch them. So, a potential attack vector would be equivocating then changing the EVM address.

On the other hand, on the state machine, we keep track of previous valsets. Thus, we should be able to identify the malicious validator, even if they change their EVM address, via checking valset.nonce = nonce - 1, or even the ones before, until the last unbonding height, and slash them accordingly.

So, editing a validator shouldn't be an issue for slashing.

Let me know what you think.

rach-id added a commit that referenced this issue Jan 6, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Closes: #768
<!-- 
Please provide an explanation of the PR, including the apprioprate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords

Co-authored-by: Rootul P <rootulp@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers testing items that are strictly related to adding or extending test coverage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants