ERC20 Approval Cap and Expiration Configurables #6521
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'openzeppelin-solidity': minor | ||
| --- | ||
|
|
||
| Added ERC20 extension for configurable approval caps and expiry |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,122 +1,155 @@ | ||
| # ERC20SafeApproval — OpenZeppelin Extension | ||
|
|
||
| > A contribution-quality extension of OpenZeppelin's ERC20 contract that adds | ||
| > configurable approval caps and per-approval expiration timestamps. | ||
|
|
||
| **Team:** Kenny Bartel & Brianna Patten | ||
| **Course:** CS5833 | ||
| **Upstream Repo:** [OpenZeppelin/openzeppelin-contracts](https://github.com/OpenZeppelin/openzeppelin-contracts) | ||
| **Related Issue:** [openzeppelin-contracts#6500](https://github.com/OpenZeppelin/openzeppelin-contracts/issues/6500) | ||
|
|
||
| --- | ||
|
|
||
| ## Overview | ||
|
|
||
| The standard ERC-20 approval mechanism allows unlimited, non-expiring approvals. | ||
| This has been a known vector for token drains in DeFi, where users who previously | ||
| signed unlimited approvals to contracts that later turned malicious had their tokens | ||
| drained. While OpenZeppelin's `ERC20Permit` (ERC-2612) addresses expiry via | ||
| off-chain signatures, it requires EIP-712 infrastructure not available in simpler | ||
| applications and does not address approval caps. | ||
|
|
||
| `ERC20SafeApproval` is an abstract Solidity extension that enforces both on-chain | ||
| through simple inheritance, requiring no off-chain signing infrastructure. | ||
|
|
||
| --- | ||
|
|
||
| ## Features | ||
|
|
||
| - **Approval Cap** — a configurable maximum approval ceiling; no spender can ever | ||
| be approved above this limit regardless of what the user signs | ||
| - **Approval Expiry** — per-approval expiration timestamps; expired approvals | ||
| automatically revert in `transferFrom()` | ||
|
|
||
| --- | ||
|
|
||
| ## Project Structure | ||
| contracts/ | ||
| └── token/ | ||
| └── ERC20/ | ||
| └── extensions/ | ||
| └── ERC20SafeApproval.sol # main extension | ||
| test/ | ||
| └── token/ | ||
| └── ERC20/ | ||
| └── ERC20SafeApproval.t.sol # Foundry test suite | ||
| script/ | ||
| └── DeployERC20SafeApproval.s.sol # Sepolia deployment script | ||
|
|
||
| --- | ||
|
|
||
| ## Getting Started | ||
|
|
||
| ### Prerequisites | ||
|
|
||
| - [Foundry](https://book.getfoundry.sh/getting-started/installation) | ||
| - Git | ||
|
|
||
| ### Installation | ||
|
|
||
| ```bash | ||
| git clone https://github.com/kennyb66/CS5833-FinalProject | ||
| cd contracts | ||
| forge install | ||
| ``` | ||
|
|
||
| ### Build | ||
|
|
||
| ```bash | ||
| forge build | ||
| ``` | ||
|
|
||
| ### Run Tests | ||
|
|
||
| ```bash | ||
| forge test --match-path test/token/ERC20/ERC20SafeApproval.t.sol -v | ||
| ``` | ||
|
Comment on lines
+70
to
+72
Contributor
There was a problem hiding this comment. Align test instructions with the actual maintained test suite. The README points to a Foundry test file path that appears to be placeholder-oriented, while this PR’s main coverage is described under the JavaScript suite. Update the “Project Structure” and “Run Tests” sections so contributors execute the real tests. Suggested doc fix-└── ERC20SafeApproval.t.sol # Foundry test suite
+└── extensions/
+ └── ERC20SafeApproval.test.js # Hardhat test suite-### Run Tests
+### Run Tests (Hardhat)
```bash
-forge test --match-path test/token/ERC20/ERC20SafeApproval.t.sol -v
+npm test -- test/token/ERC20/extensions/ERC20SafeApproval.test.jsVerify each finding against current code. Fix only still-valid issues, skip the In |
||
|
|
||
| ### Run Gas Benchmarks | ||
|
|
||
| ```bash | ||
| forge test --match-path test/token/ERC20/ERC20SafeApproval.t.sol --gas-report | ||
| ``` | ||
|
|
||
| ### Deploy to Sepolia | ||
|
|
||
| ```bash | ||
| forge script script/DeployERC20SafeApproval.s.sol \ | ||
| --rpc-url $SEPOLIA_RPC_URL \ | ||
| --private-key $PRIVATE_KEY \ | ||
| --broadcast | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Testnet Deployment | ||
|
|
||
| | Contract | Network | Address | | ||
| |----------|---------|---------| | ||
| | ERC20SafeApproval | Sepolia | `0x...` | | ||
|
|
||
| Verified on [Sepolia Etherscan](https://sepolia.etherscan.io/address/0x...) | ||
|
|
||
| --- | ||
|
|
||
| ## Gas Benchmarks | ||
|
|
||
| | Function | Base ERC20 | ERC20SafeApproval | Delta | | ||
| |----------|-----------|-------------------|-------| | ||
| | `approve()` | - | - | - | | ||
| | `transferFrom()` | - | - | - | | ||
|
|
||
| *To be filled in during Week 3* | ||
|
|
||
| --- | ||
|
|
||
| ## Usage Example | ||
|
|
||
| ```solidity | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.20; | ||
|
|
||
| import "./extensions/ERC20SafeApproval.sol"; | ||
|
|
||
| contract MyToken is ERC20SafeApproval { | ||
| constructor() ERC20("MyToken", "MTK") { | ||
| // set approval cap to 1000 tokens | ||
| // set default expiry to 30 days | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Team Contributions | ||
|
|
||
| | Member | Contributions | | ||
| |--------|--------------| | ||
| | Kenny Bartel | | | ||
| | Brianna Patten | | | ||
|
|
||
| *To be filled in as work progresses* | ||
|
|
||
| --- | ||
|
|
||
| ## Demo Video | ||
|
|
||
| *Link to be added in Week 3* | ||
|
|
||
| --- | ||
|
|
||
| ## References | ||
|
|
||
| - [OpenZeppelin ERC20 source](https://github.com/OpenZeppelin/openzeppelin-contracts/tree/master/contracts/token/ERC20) | ||
| - [ERC-2612 / ERC20Permit](https://eips.ethereum.org/EIPS/eip-2612) | ||
| - [OpenZeppelin contribution guidelines](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/CONTRIBUTING.md) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // SPDX-License-Identifier: UNLICENSED | ||
|
|
||
| pragma solidity ^0.8.20; | ||
|
|
||
| import {ERC20SafeApproval} from "../../token/ERC20/extensions/ERC20SafeApproval.sol"; | ||
| import {ERC20} from "../../token/ERC20/ERC20.sol"; | ||
|
|
||
| contract ERC20SafeApprovalMock is ERC20SafeApproval { | ||
| constructor(string memory name, string memory symbol, uint256 cap) | ||
| ERC20(name, symbol) | ||
| ERC20SafeApproval(cap) | ||
| {} | ||
|
|
||
| function mint(address account, uint256 amount) external { | ||
| _mint(account, amount); | ||
| } | ||
|
|
||
| function setApprovalCap(uint256 newCap) external { | ||
| _setApprovalCap(newCap); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,92 @@ | ||||||||||||||||||||||||
| // SPDX-License-Identifier: MIT | ||||||||||||||||||||||||
| pragma solidity ^0.8.20; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| import {ERC20} from "../ERC20.sol"; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @dev Extension of ERC20 that adds a configurable approval cap | ||||||||||||||||||||||||
| * and per-approval expiration timestamps. | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| abstract contract ERC20SafeApproval is ERC20 { | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @dev The approval amount exceeds the configured cap | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| error ERC20ApprovalCapExceeded(uint256 attempted, uint256 cap); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @dev A transferFrom was attempted with an expired approval | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| error ERC20ApprovalExpired(address owner, address spender, uint256 expiryTime); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||
| * @dev An approval was attempted with an expiration time in the past | ||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||
| error ERC20InvalidExpiration(uint256 expiryTime); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // TODO: approval cap storage | ||||||||||||||||||||||||
| uint256 private _approvalCap; | ||||||||||||||||||||||||
| // TODO: expiry storage | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // TODO: override transferFrom() | ||||||||||||||||||||||||
| mapping(address owner => mapping(address spender => uint256 expiry)) private _approvalExpiry; | ||||||||||||||||||||||||
|
Comment on lines
+27
to
+32
Contributor
There was a problem hiding this comment. Stale TODOs interleaved with implemented storage declarations. Lines 27, 29, and 31 are leftover scaffolding TODOs ( 🧹 Proposed cleanup- // TODO: approval cap storage
uint256 private _approvalCap;
- // TODO: expiry storage
-
- // TODO: override transferFrom()
mapping(address owner => mapping(address spender => uint256 expiry)) private _approvalExpiry;📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| event ApprovalCapUpdated(uint256 oldCap, uint256 newCap); | ||||||||||||||||||||||||
| event ApprovalWithExpiration(address indexed owner, address indexed spender, uint256 value, uint256 expiryTime); | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| constructor(uint256 cap) { | ||||||||||||||||||||||||
| _setApprovalCap(cap); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function getApprovalCap() public view virtual returns (uint256) { | ||||||||||||||||||||||||
| return _approvalCap; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function getApprovalExpiry(address owner, address spender) public view virtual returns (uint256) { | ||||||||||||||||||||||||
| return _approvalExpiry[owner][spender]; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function isApprovalExpired(address owner, address spender) public view virtual returns (bool) { | ||||||||||||||||||||||||
| uint256 expiry = _approvalExpiry[owner][spender]; | ||||||||||||||||||||||||
| return expiry != 0 && block.timestamp > expiry; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // TODO: override approve() | ||||||||||||||||||||||||
| function approve(address spender, uint256 amount) public virtual override returns (bool) { | ||||||||||||||||||||||||
| _checkCap(amount); | ||||||||||||||||||||||||
| delete _approvalExpiry[msg.sender][spender]; | ||||||||||||||||||||||||
| return super.approve(spender, amount); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+54
to
+59
Contributor
There was a problem hiding this comment. Remove stale Same scaffolding cleanup as above; the override is right under the TODO. Drop the comment so it doesn't mislead future readers into thinking work is pending. 🧹 Proposed cleanup- // TODO: override approve()
function approve(address spender, uint256 amount) public virtual override returns (bool) {📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function approveWithExpiration(address spender, uint256 amount, uint256 expiryTime) public virtual returns (bool) { | ||||||||||||||||||||||||
| if (expiryTime <= block.timestamp) { | ||||||||||||||||||||||||
| revert ERC20InvalidExpiration(expiryTime); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| _checkCap(amount); | ||||||||||||||||||||||||
| _approvalExpiry[msg.sender][spender] = expiryTime; | ||||||||||||||||||||||||
| emit ApprovalWithExpiration(msg.sender, spender, amount, expiryTime); | ||||||||||||||||||||||||
| return super.approve(spender, amount); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+55
to
+69
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Confirm OZ v5 ERC20 still exposes the 4-arg internal _approve and how _spendAllowance calls it
ast-grep --pattern 'function _approve(address $_, address $_, uint256 $_, bool $_) internal virtual { $$$ }'
rg -nP --type=sol -C3 '_approve\([^)]*\b(true|false)\b\s*\)' -g 'contracts/token/ERC20/**'Repository: OpenZeppelin/openzeppelin-contracts Length of output: 994 🏁 Script executed: #!/bin/bash
# Find and read ERC20SafeApproval.sol
find . -name "ERC20SafeApproval.sol" -type fRepository: OpenZeppelin/openzeppelin-contracts Length of output: 136 🏁 Script executed: #!/bin/bash
# Check the full ERC20SafeApproval.sol implementation
cat -n contracts/token/ERC20/extensions/ERC20SafeApproval.solRepository: OpenZeppelin/openzeppelin-contracts Length of output: 3900 🏁 Script executed: #!/bin/bash
# Check ERC20Permit.permit() to see how it invokes _approve
rg -n "permit" contracts/token/ERC20/extensions/ERC20Permit.sol -A 20 | head -50Repository: OpenZeppelin/openzeppelin-contracts Length of output: 1730 🏁 Script executed: #!/bin/bash
# Check _spendAllowance implementation and how it calls _approve
rg -n "_spendAllowance" contracts/token/ERC20/ERC20.sol -A 15Repository: OpenZeppelin/openzeppelin-contracts Length of output: 2054 🏁 Script executed: #!/bin/bash
# Get the complete permit() function including the _approve call
rg -n "function permit" contracts/token/ERC20/extensions/ERC20Permit.sol -A 30Repository: OpenZeppelin/openzeppelin-contracts Length of output: 1065 🏁 Script executed: #!/bin/bash
# Also verify if ERC20SafeApproval might be used with ERC20Permit in tests
find . -name "*.sol" -type f -exec grep -l "ERC20SafeApproval" {} \; | head -10Repository: OpenZeppelin/openzeppelin-contracts Length of output: 291 🏁 Script executed: #!/bin/bash
# Check if there's a 3-arg _approve method or how it resolves
rg -n "function _approve" contracts/token/ERC20/ERC20.solRepository: OpenZeppelin/openzeppelin-contracts Length of output: 381 🏁 Script executed: #!/bin/bash
# Check the test file to see if ERC20SafeApproval is tested with permit
cat -n test/token/ERC20/ERC20SafeApproval.t.sol | head -100Repository: OpenZeppelin/openzeppelin-contracts Length of output: 1113 🏁 Script executed: #!/bin/bash
# Check the implementation of the 3-arg _approve
sed -n '251,265p' contracts/token/ERC20/ERC20.solRepository: OpenZeppelin/openzeppelin-contracts Length of output: 813 🏁 Script executed: #!/bin/bash
# Verify the relationship between 3-arg and 4-arg _approve and how permit uses them
rg -n "function _approve" contracts/token/ERC20/ERC20.sol -A 5Repository: OpenZeppelin/openzeppelin-contracts Length of output: 928 Cap bypass via
Since the PR aims to provide an approval-cap alternative that complements ERC-2612, this gap should be closed in the extension itself. Override the internal virtual 4-arg 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) { | ||||||||||||||||||||||||
| uint256 expiry = _approvalExpiry[from][msg.sender]; | ||||||||||||||||||||||||
| if (expiry != 0 && block.timestamp > expiry) { | ||||||||||||||||||||||||
| revert ERC20ApprovalExpired(from, msg.sender, expiry); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| return super.transferFrom(from, to, amount); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+55
to
+77
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check which sender API is used by other ERC20 extensions in OpenZeppelin contracts
rg -nP --type=sol -C2 '\b(msg\.sender|_msgSender\(\))' -g 'contracts/token/ERC20/**'Repository: OpenZeppelin/openzeppelin-contracts Length of output: 107 🏁 Script executed: #!/bin/bash
# Find ERC20 extension files first
find contracts/token/ERC20/extensions -name "*.sol" -type f | head -20Repository: OpenZeppelin/openzeppelin-contracts Length of output: 914 🏁 Script executed: #!/bin/bash
# Check which sender API is used in ERC20 extensions
rg -n --glob='contracts/token/ERC20/**/*.sol' '(msg\.sender|_msgSender\(\))' -C 1Repository: OpenZeppelin/openzeppelin-contracts Length of output: 8849 🏁 Script executed: #!/bin/bash
# Check the base ERC20 contract's implementation
rg -n --glob='contracts/token/ERC20/ERC20.sol' '(msg\.sender|_msgSender\(\))' -B 1 -A 1Repository: OpenZeppelin/openzeppelin-contracts Length of output: 914 Use OpenZeppelin's
All other ERC20 extensions (ERC20Wrapper, ERC4626, ERC20Burnable, ERC1363, etc.) consistently use Proposed fix function approve(address spender, uint256 amount) public virtual override returns (bool) {
_checkCap(amount);
- delete _approvalExpiry[msg.sender][spender];
+ address owner = _msgSender();
+ delete _approvalExpiry[owner][spender];
return super.approve(spender, amount);
}
function approveWithExpiration(address spender, uint256 amount, uint256 expiryTime) public virtual returns (bool) {
if (expiryTime <= block.timestamp) {
revert ERC20InvalidExpiration(expiryTime);
}
_checkCap(amount);
- _approvalExpiry[msg.sender][spender] = expiryTime;
- emit ApprovalWithExpiration(msg.sender, spender, amount, expiryTime);
+ address owner = _msgSender();
+ _approvalExpiry[owner][spender] = expiryTime;
+ emit ApprovalWithExpiration(owner, spender, amount, expiryTime);
return super.approve(spender, amount);
}
function transferFrom(address from, address to, uint256 amount) public virtual override returns (bool) {
- uint256 expiry = _approvalExpiry[from][msg.sender];
+ address spender = _msgSender();
+ uint256 expiry = _approvalExpiry[from][spender];
if (expiry != 0 && block.timestamp > expiry) {
- revert ERC20ApprovalExpired(from, msg.sender, expiry);
+ revert ERC20ApprovalExpired(from, spender, expiry);
}
return super.transferFrom(from, to, amount);
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function _checkCap(uint256 value) internal view virtual { | ||||||||||||||||||||||||
| uint256 cap = _approvalCap; | ||||||||||||||||||||||||
| if (cap != type(uint256).max && value > cap) { | ||||||||||||||||||||||||
| revert ERC20ApprovalCapExceeded(value, cap); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| function _setApprovalCap(uint256 newCap) internal virtual { | ||||||||||||||||||||||||
| uint256 oldCap = _approvalCap; | ||||||||||||||||||||||||
| _approvalCap = newCap; | ||||||||||||||||||||||||
| emit ApprovalCapUpdated(oldCap, newCap); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.20; | ||
|
|
||
| import {ERC20SafeApproval} from "./ERC20SafeApproval.sol"; | ||
| import {ERC20} from "../ERC20.sol"; | ||
|
|
||
| /** | ||
| * @dev Concrete ERC20 token using ERC20SafeApproval extension. | ||
| * Used for testing and demonstration purposes. | ||
| */ | ||
| contract ERC20SafeApprovalToken is ERC20SafeApproval { | ||
| constructor(string memory name, string memory symbol, uint256 cap, uint256 initialSupply) | ||
| ERC20(name, symbol) | ||
| ERC20SafeApproval(cap) | ||
| { | ||
| _mint(msg.sender, initialSupply); | ||
| } | ||
|
|
||
| // TODO: add owner storage and onlyOwner modifier | ||
|
|
||
| // TODO: add constructor | ||
|
|
||
| // TODO: expose setApprovalCap for owner | ||
|
Comment on lines
+19
to
+23
Contributor
There was a problem hiding this comment. Reconcile/clean up the trailing TODOs.
Either resolution is fine, but the current "TODO list as a comment" leaves the concrete token in an in-between state. Want me to draft the 🤖 Prompt for AI Agents |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix repository setup path in installation steps.
The
cd contractsstep is likely incorrect for project setup and can breakforge installfor users following the README. Use the repository root (or the actual directory containingfoundry.toml) instead.Suggested doc fix
📝 Committable suggestion
🤖 Prompt for AI Agents