diff --git a/docs/src/content/extensions/accounting.md b/docs/src/content/extensions/accounting.md index bcd1ba87..387cb805 100644 --- a/docs/src/content/extensions/accounting.md +++ b/docs/src/content/extensions/accounting.md @@ -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. @@ -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. diff --git a/docs/src/content/extensions/bond_escalation_accounting.md b/docs/src/content/extensions/bond_escalation_accounting.md index 3c188103..58111b9c 100644 --- a/docs/src/content/extensions/bond_escalation_accounting.md +++ b/docs/src/content/extensions/bond_escalation_accounting.md @@ -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. diff --git a/solidity/contracts/extensions/AccountingExtension.sol b/solidity/contracts/extensions/AccountingExtension.sol index 544e5c27..7f6af145 100644 --- a/solidity/contracts/extensions/AccountingExtension.sol +++ b/solidity/contracts/extensions/AccountingExtension.sol @@ -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); } diff --git a/solidity/interfaces/extensions/IAccountingExtension.sol b/solidity/interfaces/extensions/IAccountingExtension.sol index 223b55a0..ad2d11bf 100644 --- a/solidity/interfaces/extensions/IAccountingExtension.sol +++ b/solidity/interfaces/extensions/IAccountingExtension.sol @@ -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 @@ -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 */ diff --git a/solidity/test/unit/extensions/AccountingExtension.t.sol b/solidity/test/unit/extensions/AccountingExtension.t.sol index 7234222d..cc4be92b 100644 --- a/solidity/test/unit/extensions/AccountingExtension.t.sol +++ b/solidity/test/unit/extensions/AccountingExtension.t.sol @@ -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); @@ -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 */