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.

Sign up for GitHub

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

Add token wrapper #526

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from
Draft

Add token wrapper #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14c34d4
Add token wrapper
ben-kaufman Aug 18, 2023
703857e
Upgrade OZ
ben-kaufman Aug 18, 2023
f1052a0
Update docs
ben-kaufman Aug 18, 2023
b8b1fd6
Lint
ben-kaufman Aug 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions contracts/HATTokenWrapper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// SPDX-License-Identifier: MIT
// Disclaimer https://github.com/hats-finance/hats-contracts/blob/main/DISCLAIMER.md

pragma solidity 0.8.16;

import "@openzeppelin/contracts/token/ERC20/extensions/ERC4626.sol";

contract HATTokenWrapper is ERC4626 {

error AmountOfSharesMustBeMoreThanMinimalAmount();

uint256 public constant MINIMAL_AMOUNT_OF_SHARES = 1e3; // to reduce rounding errors, the number of shares is either 0, or > than this number

constructor (IERC20 _asset, string memory _name, string memory _symbol) ERC4626(_asset) ERC20(_name, _symbol) {}

function _afterTokenTransfer(address, address, uint256) internal virtual override {
if (totalSupply() > 0 && totalSupply() < MINIMAL_AMOUNT_OF_SHARES) {
revert AmountOfSharesMustBeMoreThanMinimalAmount();
}
}

}
Comment on lines +1 to +22
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The HATTokenWrapper contract seems to be well implemented. It correctly extends the ERC4626 contract from OpenZeppelin and overrides the _afterTokenTransfer function to enforce a minimum amount of shares. The use of custom error types for revert reasons is also a good practice as it can make debugging easier. However, there's a minor issue with the constructor. It should call the parent constructors in the correct order.

-    constructor (IERC20 _asset, string memory _name, string memory _symbol) ERC4626(_asset) ERC20(_name, _symbol) {}
+    constructor (IERC20 _asset, string memory _name, string memory _symbol) ERC20(_name, _symbol) ERC4626(_asset) {}

2 changes: 1 addition & 1 deletion contracts/HATVault.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -782,7 +782,7 @@ contract HATVault is IHATVault, ERC4626Upgradeable, OwnableUpgradeable, Reentran
if (_from == _to) revert CannotTransferToSelf();
// deposit/mint/transfer
if (_to != address(0)) {
HATVaultsRegistry _registry = registry;
HATVaultsRegistry _registry = registry;
if (_registry.isEmergencyPaused()) revert SystemInEmergencyPause();
// Cannot transfer or mint tokens to a user for which an active withdraw request exists
// because then we would need to reset their withdraw request
Expand Down
3 changes: 1 addition & 2 deletions contracts/interfaces/IHATToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@
pragma solidity 0.8.16;

import "@openzeppelin/contracts/governance/utils/IVotes.sol";
import "@openzeppelin/contracts/token/ERC20/extensions/draft-IERC20Permit.sol";

interface IHATToken is IVotes, IERC20Permit {
interface IHATToken is IVotes {
Comment on lines 5 to +6
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The IHATToken interface no longer extends the IERC20Permit interface. This change could potentially break any contracts or off-chain services that rely on the permit functionality of the IHATToken. If this is intentional and you've considered the implications, then it's fine. However, if the permit functionality is still needed, you should reconsider this change.

- interface IHATToken is IVotes {
+ interface IHATToken is IVotes, IERC20Permit {


// Amount for minting or burning cannot be zero
error ZeroAmount();
Expand Down
2 changes: 1 addition & 1 deletion docs/dodoc/HATGovernanceArbitrator.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function renounceOwnership() external nonpayable



*Leaves the contract without owner. It will not be possible to call `onlyOwner` functions anymore. Can only be called by the current owner. NOTE: Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.*
*Leaves the contract without owner. It will not be possible to call `onlyOwner` functions. Can only be called by the current owner. NOTE: Renouncing ownership will leave the contract without an owner, thereby disabling any functionality that is only available to the owner.*


### transferOwnership
Expand Down
2 changes: 1 addition & 1 deletion docs/dodoc/HATHackersNFT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ function renounceOwnership() external nonpayable



*Leaves the contract without owner. It will not be possible to call `onlyOwner` functions anymore. Can only be called by the current owner. NOTE: Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.*
*Leaves the contract without owner. It will not be possible to call `onlyOwner` functions. Can only be called by the current owner. NOTE: Renouncing ownership will leave the contract without an owner, thereby disabling any functionality that is only available to the owner.*


### safeBatchTransferFrom
Expand Down
59 changes: 38 additions & 21 deletions docs/dodoc/HATTimelockController.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ function executeBatch(address[] targets, uint256[] values, bytes[] payloads, byt
### getMinDelay

```solidity
function getMinDelay() external view returns (uint256 duration)
function getMinDelay() external view returns (uint256)
```


Expand All @@ -202,7 +202,7 @@ function getMinDelay() external view returns (uint256 duration)

| Name | Type | Description |
|---|---|---|
| duration | uint256 | undefined |
| _0 | uint256 | undefined |

### getRoleAdmin

Expand All @@ -229,12 +229,12 @@ function getRoleAdmin(bytes32 role) external view returns (bytes32)
### getTimestamp

```solidity
function getTimestamp(bytes32 id) external view returns (uint256 timestamp)
function getTimestamp(bytes32 id) external view returns (uint256)
```



*Returns the timestamp at with an operation becomes ready (0 for unset operations, 1 for done operations).*
*Returns the timestamp at which an operation becomes ready (0 for unset operations, 1 for done operations).*

#### Parameters

Expand All @@ -246,7 +246,7 @@ function getTimestamp(bytes32 id) external view returns (uint256 timestamp)

| Name | Type | Description |
|---|---|---|
| timestamp | uint256 | undefined |
| _0 | uint256 | undefined |

### grantRole

Expand Down Expand Up @@ -291,7 +291,7 @@ function hasRole(bytes32 role, address account) external view returns (bool)
### hashOperation

```solidity
function hashOperation(address target, uint256 value, bytes data, bytes32 predecessor, bytes32 salt) external pure returns (bytes32 hash)
function hashOperation(address target, uint256 value, bytes data, bytes32 predecessor, bytes32 salt) external pure returns (bytes32)
```


Expand All @@ -312,12 +312,12 @@ function hashOperation(address target, uint256 value, bytes data, bytes32 predec

| Name | Type | Description |
|---|---|---|
| hash | bytes32 | undefined |
| _0 | bytes32 | undefined |

### hashOperationBatch

```solidity
function hashOperationBatch(address[] targets, uint256[] values, bytes[] payloads, bytes32 predecessor, bytes32 salt) external pure returns (bytes32 hash)
function hashOperationBatch(address[] targets, uint256[] values, bytes[] payloads, bytes32 predecessor, bytes32 salt) external pure returns (bytes32)
```


Expand All @@ -338,12 +338,12 @@ function hashOperationBatch(address[] targets, uint256[] values, bytes[] payload

| Name | Type | Description |
|---|---|---|
| hash | bytes32 | undefined |
| _0 | bytes32 | undefined |

### isOperation

```solidity
function isOperation(bytes32 id) external view returns (bool registered)
function isOperation(bytes32 id) external view returns (bool)
```


Expand All @@ -360,12 +360,12 @@ function isOperation(bytes32 id) external view returns (bool registered)

| Name | Type | Description |
|---|---|---|
| registered | bool | undefined |
| _0 | bool | undefined |

### isOperationDone

```solidity
function isOperationDone(bytes32 id) external view returns (bool done)
function isOperationDone(bytes32 id) external view returns (bool)
```


Expand All @@ -382,17 +382,17 @@ function isOperationDone(bytes32 id) external view returns (bool done)

| Name | Type | Description |
|---|---|---|
| done | bool | undefined |
| _0 | bool | undefined |

### isOperationPending

```solidity
function isOperationPending(bytes32 id) external view returns (bool pending)
function isOperationPending(bytes32 id) external view returns (bool)
```



*Returns whether an operation is pending or not.*
*Returns whether an operation is pending or not. Note that a &quot;pending&quot; operation may also be &quot;ready&quot;.*

#### Parameters

Expand All @@ -404,17 +404,17 @@ function isOperationPending(bytes32 id) external view returns (bool pending)

| Name | Type | Description |
|---|---|---|
| pending | bool | undefined |
| _0 | bool | undefined |

### isOperationReady

```solidity
function isOperationReady(bytes32 id) external view returns (bool ready)
function isOperationReady(bytes32 id) external view returns (bool)
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The return type of the isOperationReady function has been changed from (bool ready) to (bool). This change removes the explicit variable name from the return type, which might make it less clear for users of this function what the returned value represents. Consider adding a comment above the function definition to explain what the returned bool represents.

- function isOperationReady(bytes32 id) external view returns (bool)
+ // Returns true if the operation is ready, false otherwise
+ function isOperationReady(bytes32 id) external view returns (bool ready)

```



*Returns whether an operation is ready or not.*
*Returns whether an operation is ready for execution. Note that a &quot;ready&quot; operation is also &quot;pending&quot;.*

#### Parameters

Expand All @@ -426,7 +426,7 @@ function isOperationReady(bytes32 id) external view returns (bool ready)

| Name | Type | Description |
|---|---|---|
| ready | bool | undefined |
| _0 | bool | undefined |

### onERC1155BatchReceived

Expand Down Expand Up @@ -547,7 +547,7 @@ function schedule(address target, uint256 value, bytes data, bytes32 predecessor



*Schedule an operation containing a single transaction. Emits a {CallScheduled} event. Requirements: - the caller must have the &#39;proposer&#39; role.*
*Schedule an operation containing a single transaction. Emits {CallSalt} if salt is nonzero, and {CallScheduled}. Requirements: - the caller must have the &#39;proposer&#39; role.*
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

The documentation for the scheduling functions now mentions that a CallSalt event will be emitted if the salt is nonzero. This is a useful addition as it provides more information about the behavior of these functions.


#### Parameters

Expand All @@ -568,7 +568,7 @@ function scheduleBatch(address[] targets, uint256[] values, bytes[] payloads, by



*Schedule an operation containing a batch of transactions. Emits one {CallScheduled} event per transaction in the batch. Requirements: - the caller must have the &#39;proposer&#39; role.*
*Schedule an operation containing a batch of transactions. Emits {CallSalt} if salt is nonzero, and one {CallScheduled} event per transaction in the batch. Requirements: - the caller must have the &#39;proposer&#39; role.*

#### Parameters

Expand Down Expand Up @@ -767,6 +767,23 @@ event CallExecuted(bytes32 indexed id, uint256 indexed index, address target, ui
| value | uint256 | undefined |
| data | bytes | undefined |

### CallSalt

```solidity
event CallSalt(bytes32 indexed id, bytes32 salt)
```





#### Parameters

| Name | Type | Description |
|---|---|---|
| id `indexed` | bytes32 | undefined |
| salt | bytes32 | undefined |

Comment on lines +773 to +786
Copy link

Choose a reason for hiding this comment

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

Image description CodeRabbit

A new event CallSalt has been added. This event is emitted when an operation with a non-zero salt is scheduled. This is a good addition as it allows tracking of operations with specific salts.

+ // Emitted when an operation with a non-zero salt is scheduled
+ event CallSalt(bytes32 indexed id, bytes32 salt)

### CallScheduled

```solidity
Expand Down
Loading