Skip to content

Commit

Permalink
fix: fee-on-transfer token
Browse files Browse the repository at this point in the history
  • Loading branch information
0xJabberwock committed Jul 8, 2024
1 parent 8a5f3f3 commit 340f9ce
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 4 deletions.
6 changes: 3 additions & 3 deletions docs/src/content/extensions/accounting.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ The Accounting Extension is a contract that allows users to deposit and bond fun

## 3. Key Mechanisms & Concepts

- **Deposits**: Users can deposit tokens into the Accounting Extension. These deposits are tracked in a mapping that associates each user's address with their balance of each token. Deposits can be made in any ERC20 token, including wrapped Ether (WETH).
- **Deposits**: Users can deposit tokens into the Accounting Extension. These deposits are tracked in a mapping that associates each user's address with their balance of each token. Deposits can be made in many ERC20 tokens, including wrapped Ether (WETH).

- **Withdrawals**: Users can withdraw their deposited tokens at any time, provided they have sufficient balance. The withdrawal operation reduces the user's balance in the Accounting Extension and transfers the tokens back to the user's address. Locked tokens can't be withdrawn until they're released by a module.

Expand All @@ -31,5 +31,5 @@ The Accounting Extension is a contract that allows users to deposit and bond fun
## 4. Gotchas

- Before depositing ERC20 tokens, users must first approve the Accounting Extension to transfer the tokens on their behalf.
- Users can only withdraw tokens that are not currently bonded. If a user has bonded tokens for a request, those tokens are locked until they are released by an allowed module. Attempting to withdraw bonded tokens will result in an error. Attempting to slash or pay out tokens that are not locked will also result in a transaction being reverted.
- The contract supports any ERC20 token, including wrapped Ether (WETH). However, if a user tries to deposit a non-ERC20 token or a token that the contract otherwise doesn't support, the transaction will fail.
- Users can only withdraw tokens that are not currently bonded. If a user has bonded tokens for a request, those tokens are locked until they are released by an allowed module. Attempting to withdraw bonded tokens will result in an error. Attempting to slash or pay out tokens that are not locked will also result in a transaction being reverted.
- The contract supports many ERC20 tokens, including wrapped Ether (WETH). However, if a user tries to deposit a ERC20 token with a fee on transfer, a non-ERC20 token or a token that the contract otherwise doesn't support, the transaction will fail.
4 changes: 3 additions & 1 deletion docs/src/content/extensions/bond_escalation_accounting.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ The `BondEscalationAccounting` contract is an extension that allows users to dep

- Pledging: Users can pledge tokens for or against a dispute. The pledged tokens are locked and cannot be used until the dispute is resolved.

- Deposits: Users can deposit tokens into the extension. Deposits can be made in any ERC20 token, ETH deposits will be converted to WETH.
- Deposits: Users can deposit tokens into the extension. Deposits can be made in many ERC20 tokens, ETH deposits will be converted to WETH.

- Withdrawals: Users can withdraw their tokens at any time. The withdrawal operation reduces the user's balance in the extension and transfers the tokens back to the user's address. Locked tokens can't be withdrawn until they're released by a module.

## 4. Gotchas

- Before depositing ERC20 tokens, users must first approve the extension to transfer the tokens on their behalf.
- Users can only withdraw tokens that are not currently bonded. If a user has bonded tokens for a request, those tokens are locked until they are released by an allowed module. Attempting to withdraw bonded tokens will result in an error. Attempting to slash or pay out tokens that are not locked will also result in a transaction being reverted.
- The contract supports many ERC20 tokens, including wrapped Ether (WETH). However, if a user tries to deposit a ERC20 token with a fee on transfer, a non-ERC20 token or a token that the contract otherwise doesn't support, the transaction will fail.
1 change: 1 addition & 0 deletions solidity/contracts/extensions/AccountingExtension.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ contract AccountingExtension is IAccountingExtension {
/// @inheritdoc IAccountingExtension
function deposit(IERC20 _token, uint256 _amount) external {
_token.safeTransferFrom(msg.sender, address(this), _amount);
if (_amount != _token.balanceOf(address(this))) revert AccountingExtension_FeeOnTransferToken();
balanceOf[msg.sender][_token] += _amount;
emit Deposited(msg.sender, _token, _amount);
}
Expand Down
6 changes: 6 additions & 0 deletions solidity/interfaces/extensions/IAccountingExtension.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ interface IAccountingExtension {
ERRORS
//////////////////////////////////////////////////////////////*/

/**
* @notice Thrown when depositing tokens with a fee on transfer
*/
error AccountingExtension_FeeOnTransferToken();

/**
* @notice Thrown when the account doesn't have enough balance to bond/withdraw
* or not enough bonded to release/pay
Expand Down Expand Up @@ -121,6 +126,7 @@ interface IAccountingExtension {
/**
* @notice Transfers tokens from a user and updates his virtual balance
* @dev The user must have approved the accounting extension to transfer the tokens.
* The transfer must not take a fee (deflationary tokens can lead to unexpected behavior).
* @param _token The address of the token being deposited
* @param _amount The amount of `_token` to deposit
*/
Expand Down
24 changes: 24 additions & 0 deletions solidity/test/unit/extensions/AccountingExtension.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ contract AccountingExtension_Unit_DepositAndWithdraw is BaseTest {
address(token), abi.encodeCall(IERC20.transferFrom, (sender, address(extension), _amount)), abi.encode(true)
);

// Mock and expect the ERC20 balance
_mockAndExpect(address(token), abi.encodeCall(IERC20.balanceOf, (address(extension))), abi.encode(_amount));

// Expect the event
vm.expectEmit(true, true, true, true, address(extension));
emit Deposited(sender, token, _amount);
Expand All @@ -82,6 +85,27 @@ contract AccountingExtension_Unit_DepositAndWithdraw is BaseTest {
assertEq(extension.balanceOf(sender, token), _amount);
}

/**
* @notice Should revert if token takes a fee on transfer
*/
function test_depositRevert(uint256 _amount, uint256 _fee) public {
vm.assume(_amount > _fee);
vm.assume(_fee > 0);

// Mock and expect the ERC20 transfer
_mockAndExpect(
address(token), abi.encodeCall(IERC20.transferFrom, (sender, address(extension), _amount)), abi.encode(true)
);

// Mock and expect the ERC20 balance
_mockAndExpect(address(token), abi.encodeCall(IERC20.balanceOf, (address(extension))), abi.encode(_amount - _fee));

// Check: does it revert if token takes a fee on transfer?
vm.expectRevert(IAccountingExtension.AccountingExtension_FeeOnTransferToken.selector);
vm.prank(sender);
extension.deposit(token, _amount);
}

/**
* @notice Test withdrawing ERC20. Should update balance and emit event
*/
Expand Down

0 comments on commit 340f9ce

Please sign in to comment.