Conversation
Feat/token swap coinvestor
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| "@openzeppelin/contracts": "4.9.6", | ||
| "@openzeppelin/contracts-upgradeable": "4.9.6" | ||
| "@openzeppelin/contracts-upgradeable": "4.9.6", | ||
| "@safe-global/safe-contracts": "^1.4.1-2" |
There was a problem hiding this comment.
Currently, we deploy SafeContracts v1.3. But this shouldn't be an issue.
contracts/Distribution.sol
Outdated
| } | ||
|
|
||
| function claim(IERC1271 _holder, bytes32 _hash, bytes memory _signature, address _recipient) external { | ||
| require(_holder.isValidSignature(_hash, _signature) == 0x1626ba7e); |
There was a problem hiding this comment.
On Slack you said this is not complete. What is missing? Isn't this enough?
There was a problem hiding this comment.
Technical:
While this satisfies the standard, it
- doesn's have replay protection
- doesn't enforce _recipient being part of the hash, so adversaries monitoring the mempool could replace _recipient and steal the funds
- possibly more, as I haven't studied it at depth yet
Practical:
It is unclear in which situation this function would be beneficial over safe.execTransaction(), so I can't assess if it actually delivers that expected value.
Safe.execTransaction is the tried and tested, general-purpose solution, so ECR1271 would have to provide some significant benefit to justify the additional complexity and attack surface.
Any safe can claim through safe.execTransaction()
There was a problem hiding this comment.
Agreed to remove ERC1271 support in call today. c7fbc32
Changelog
Unreleased
New contract:
CoinvestedPositionHolds tokens on behalf of a co-investor and sells them, splitting proceeds between the co-investor (receiver) and lead investors via carry.
basePriceper token; any surplus is carry split among lead investors bycarryFraction. If net proceeds don't coverbasePrice, all proceeds go to the receiver.buy, dividends, exit) accept any EURO token (TRUSTED_CURRENCY | EURO_CURRENCY). The currency used bybuy()is a state variable the owner can update viasetCurrency().receiver, including any accidentally sent funds.New contract:
TokenSwapBaseAbstract base extracted from duplicated logic in
TokenSwapandCoinvestedPosition, covering shared state, fee handling, price/receiver management, pause controls, and ERC-2771 support. Both contracts now extend it.New contract:
DistributionDistributes a fixed currency amount among token holders proportional to their balance at a given snapshot. Supports direct claims, ERC-1271 smart-contract holders, and vesting contracts. An owner-only
reassignfunction (available after a configurable delay post-deployment) handles recovery cases. Deployed via an atomic clone-and-fund factory.New contract:
ExitAllows token holders to redeem tokens for a fixed currency payout within a configurable duration after the exit date. Mirrors
Distribution's claim overloads and factory pattern.New constant:
EURO_CURRENCYinAllowListuint256 constant EURO_CURRENCY = 2 ** 254(bit 254) added alongsideTRUSTED_CURRENCY(bit 255). Marks a currency as Euro-denominated; required byCoinvestedPositionfor all currency inputs.Breaking change:
TRUSTED_CURRENCYcheck relaxed to bitmaskAffected:
Crowdinvesting,PrivateOffer,TokenSwapThe currency allowList check changed from exact equality to a bitmask, allowing currency addresses to carry additional bits (e.g.
EURO_CURRENCY) without being rejected. Existing deployments are affected when the attributes on the AllowList or the AllowList itself are updated.Old Crowdinvestings and PrivateOffers will not work with the new AllowList format, as their TRUSTED_CURRENCY check will return false.