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

[delta-audit] treasury: Avoid overriding vote of non self-bonding accounts #626

Merged
merged 3 commits into from
Sep 25, 2023

Conversation

victorges
Copy link
Member

@victorges victorges commented Sep 20, 2023

What does this pull request do? Explain your changes. (required)
This fixes a finding from the audit regarding on overriding votes. It addresses the problem of delegating to a non-transcoder account and then overriding their votes imprperly.

The known corner cases that can trigger this inconsistency are:

  • delegating to an account that isn't a transcoder (either another delegator or a non participant)
  • transcoder unbonding their full amount (delegators stay delegated to it)

Specific updates (required)

  • Update GovernorCountingOverridable to not override votes of accounts that don't self-delegate

How did you test each of these updates (required)
yarn test to be deployed on devnet

Does this pull request close any open issues?
Fixes:

Checklist:

  • README and other documentation updated
  • All tests using yarn test pass

@victorges victorges changed the base branch from confluence to delta September 20, 2023 18:04
@victorges victorges marked this pull request as ready for review September 20, 2023 18:35
@victorges victorges changed the title [delta-audit] treasury: Prevent overriding the vote of non self-bonding accounts [delta-audit] treasury: Prevent overriding vote of non self-bonding accounts Sep 20, 2023
@victorges victorges changed the title [delta-audit] treasury: Prevent overriding vote of non self-bonding accounts [delta-audit] treasury: Avoid overriding vote of non self-bonding accounts Sep 20, 2023
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good and just a few comments.

contracts/treasury/GovernorCountingOverridable.sol Outdated Show resolved Hide resolved
contracts/treasury/GovernorCountingOverridable.sol Outdated Show resolved Hide resolved
contracts/treasury/GovernorCountingOverridable.sol Outdated Show resolved Hide resolved
Copy link
Member

@yondonfu yondonfu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@victorges victorges merged commit ff0bd7c into delta Sep 25, 2023
3 checks passed
@victorges victorges deleted the vg/fix/c4-audit-96-206 branch September 25, 2023 16:58
victorges added a commit that referenced this pull request Sep 27, 2023
…ounts (#626)

* treasury: Make consistent checks for isTranscoder

* test/treasury: Add a unit test to the new logic

* treasury: Fix weird variable names
victorges added a commit that referenced this pull request Oct 11, 2023
…ounts (#626)

* treasury: Make consistent checks for isTranscoder

* test/treasury: Add a unit test to the new logic

* treasury: Fix weird variable names
victorges added a commit that referenced this pull request Oct 11, 2023
…ounts (#626)

* treasury: Make consistent checks for isTranscoder

* test/treasury: Add a unit test to the new logic

* treasury: Fix weird variable names
victorges added a commit that referenced this pull request Oct 11, 2023
…ounts (#626)

* treasury: Make consistent checks for isTranscoder

* test/treasury: Add a unit test to the new logic

* treasury: Fix weird variable names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants