Delegator contract #16173
Delegator contract #16173
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a delegator contract system that allows token holders to delegate voting power to multiple addresses in a single transaction without transferring actual token ownership. The system creates individual delegation contracts that hold tokens and automatically delegate voting rights to specified addresses.
Key changes:
- Implements a factory pattern where the main Delegator contract creates individual Delegate contracts for each delegation relationship
- Provides delegation and undelegation functionality while maintaining token custody in separate contracts
- Includes comprehensive test coverage for basic delegation scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| smart-contracts/contracts/Delegator.sol | Main delegator contract with factory pattern for creating delegate contracts and managing token transfers |
| smart-contracts/test/Delegator/delegate.js | Test suite covering delegation, undelegation, and voting power verification scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| import "@openzeppelin/contracts-upgradeable5/token/ERC20/extensions/ERC20VotesUpgradeable.sol"; | ||
| import "@openzeppelin/contracts-upgradeable5/token/ERC20/ERC20Upgradeable.sol"; | ||
| import "hardhat/console.sol"; |
There was a problem hiding this comment.
The hardhat/console.sol import should be removed from production code as it's only needed for debugging during development.
| import "hardhat/console.sol"; |
| contract Delegate { | ||
| constructor(address token, address to) { | ||
| ERC20VotesUpgradeable(token).delegate(to); // Delegates to the specified address | ||
| IERC20(token).approve(msg.sender, type(uint).max); // Approve the caller to get all the tokens |
There was a problem hiding this comment.
Using type(uint).max for approval creates an unlimited allowance which poses security risks. Consider approving only the specific amount needed or implementing a more controlled approval mechanism.
| // And check delegations! | ||
| expect(await token.delegates(delegateAddress)).to.equal(ethers.ZeroAddress) | ||
| expect(await token.delegates(delegatorAddress)).to.equal(ethers.ZeroAddress) | ||
| expect(await token.delegates(ownerAddress)).to.equal(ownerAddress) // Onwer still delegates to themselves |
There was a problem hiding this comment.
Typo in comment: 'Onwer' should be 'Owner'.
| // And check the votes! | ||
| expect(await token.getVotes(delegateAddress)).to.equal(amount) | ||
| expect(await token.getVotes(delegatorAddress)).to.equal(0n) | ||
| expect(await token.getVotes(ownerAddress)).to.equal(balanceBefore - amount) // Onwer still getVotes to themselves |
There was a problem hiding this comment.
Typo in comment: 'Onwer' should be 'Owner'.
| // And check delegations! | ||
| expect(await token.delegates(delegateAddress)).to.equal(ethers.ZeroAddress) | ||
| expect(await token.delegates(delegatorAddress)).to.equal(ethers.ZeroAddress) | ||
| expect(await token.delegates(ownerAddress)).to.equal(ownerAddress) // Onwer still delegates to themselves |
There was a problem hiding this comment.
Typo in comment: 'Onwer' should be 'Owner'.
| expect(await token.getVotes(delegatorAddress)).to.equal(0n) | ||
| expect(await token.getVotes(ownerAddress)).to.equal( | ||
| balanceBefore - amount * 2n | ||
| ) // Onwer still getVotes to themselves |
There was a problem hiding this comment.
Typo in comment: 'Onwer' should be 'Owner'.
| ) // Onwer still getVotes to themselves | |
| ) // Owner still getVotes to themselves |
| await token.approve(delegatorAddress, amount) | ||
|
|
||
| const balanceBefore = await token.balanceOf(ownerAddress) | ||
|
|
||
| // Delegate the tokens | ||
| await token.approve(delegatorAddress, amount) |
There was a problem hiding this comment.
Duplicate approval on lines 107 and 112. The first approval on line 107 is redundant and should be removed.
Description
This is an early contract that acts as a "delegator" so a given wallet can delegate different amounts of tokens to various different addresses in a single tx.
We would need a front-end!
Checklist:
Release Note Draft Snippet