Skip to content

Commit 32e59ae

Browse files
kumaryash90kumaryash90
authored
Pack update: add _trustedForwarders param in initialize (#244)
* _trustedForwarders in initialize * run prettier * size and optimize
1 parent 49069e4 commit 32e59ae

File tree

8 files changed

+52
-28
lines changed

8 files changed

+52
-28
lines changed

‎contracts/extension/TokenBundle.sol‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ abstract contract TokenBundle is ITokenBundle {
3838
function _createBundle(Token[] calldata _tokensToBind, uint256 _bundleId) internal {
3939
uint256 targetCount = _tokensToBind.length;
4040

41-
require(targetCount > 0, "no tokens to bind");
42-
require(bundle[_bundleId].count == 0, "existent at bundleId");
41+
require(targetCount > 0, "!Tokens");
42+
require(bundle[_bundleId].count == 0, "id exists");
4343

4444
for (uint256 i = 0; i < targetCount; i += 1) {
4545
_checkTokenType(_tokensToBind[i]);
@@ -51,7 +51,7 @@ abstract contract TokenBundle is ITokenBundle {
5151

5252
/// @dev Lets the calling contract update a bundle, by passing in a list of tokens and a unique id.
5353
function _updateBundle(Token[] memory _tokensToBind, uint256 _bundleId) internal {
54-
require(_tokensToBind.length > 0, "no tokens to bind");
54+
require(_tokensToBind.length > 0, "!Tokens");
5555

5656
uint256 currentCount = bundle[_bundleId].count;
5757
uint256 targetCount = _tokensToBind.length;

‎contracts/pack/Pack.sol‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ contract Pack is
9292
string memory _name,
9393
string memory _symbol,
9494
string memory _contractURI,
95+
address[] memory _trustedForwarders,
9596
address _royaltyRecipient,
9697
uint256 _royaltyBps
9798
) external initializer {
@@ -101,8 +102,17 @@ contract Pack is
101102
// Initialize inherited contracts, most base-like -> most derived.
102103
__ReentrancyGuard_init();
103104

104-
address[] memory forwarders = new address[](1);
105-
forwarders[0] = forwarder;
105+
/** note: The immutable state-variable `forwarder` is an EOA-only forwarder,
106+
* which guards against automated attacks.
107+
*
108+
* Use other forwarders only if there's a strong reason to bypass this check.
109+
*/
110+
address[] memory forwarders = new address[](_trustedForwarders.length + 1);
111+
uint256 i;
112+
for (; i < _trustedForwarders.length; i++) {
113+
forwarders[i] = _trustedForwarders[i];
114+
}
115+
forwarders[i] = forwarder;
106116
__ERC2771Context_init(forwarders);
107117
__ERC1155_init(_contractURI);
108118

@@ -111,7 +121,6 @@ contract Pack is
111121

112122
_setupContractURI(_contractURI);
113123
_setupOwner(_defaultAdmin);
114-
115124
_setupRole(DEFAULT_ADMIN_ROLE, _defaultAdmin);
116125

117126
_setupRole(_transferRole, _defaultAdmin);
@@ -192,8 +201,7 @@ contract Pack is
192201
uint128 _amountDistributedPerOpen,
193202
address _recipient
194203
) external payable onlyRoleWithSwitch(minterRole) nonReentrant returns (uint256 packId, uint256 packTotalSupply) {
195-
require(_contents.length > 0, "!Contents");
196-
require(_contents.length == _numOfRewardUnits.length, "!Rewards");
204+
require(_contents.length > 0 && _contents.length == _numOfRewardUnits.length, "!Len");
197205

198206
if (!hasRole(assetRole, address(0))) {
199207
for (uint256 i = 0; i < _contents.length; i += 1) {
@@ -236,9 +244,8 @@ contract Pack is
236244
returns (uint256 packTotalSupply, uint256 newSupplyAdded)
237245
{
238246
require(canUpdatePack[_packId], "!Allowed");
239-
require(_contents.length > 0, "!Contents");
240-
require(_contents.length == _numOfRewardUnits.length, "!Rewards");
241-
require(balanceOf(_recipient, _packId) != 0, "!Recipient");
247+
require(_contents.length > 0 && _contents.length == _numOfRewardUnits.length, "!Len");
248+
require(balanceOf(_recipient, _packId) != 0, "!Bal");
242249

243250
if (!hasRole(assetRole, address(0))) {
244251
for (uint256 i = 0; i < _contents.length; i += 1) {
@@ -261,7 +268,7 @@ contract Pack is
261268
address opener = _msgSender();
262269

263270
require(isTrustedForwarder(msg.sender) || opener == tx.origin, "!EOA");
264-
require(balanceOf(opener, _packId) >= _amountToOpen, "!Balance");
271+
require(balanceOf(opener, _packId) >= _amountToOpen, "!Bal");
265272

266273
PackInfo memory pack = packInfo[_packId];
267274
require(pack.openStartTimestamp <= block.timestamp, "cant open");
@@ -290,15 +297,15 @@ contract Pack is
290297

291298
for (uint256 i = 0; i < _contents.length; i += 1) {
292299
require(_contents[i].totalAmount != 0, "0 amt");
293-
require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "!Rewards");
294-
require(_contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1, "!Rewards");
300+
require(_contents[i].totalAmount % _numOfRewardUnits[i] == 0, "!R");
301+
require(_contents[i].tokenType != TokenType.ERC721 || _contents[i].totalAmount == 1, "!R");
295302

296303
sumOfRewardUnits += _numOfRewardUnits[i];
297304

298305
packInfo[packId].perUnitAmounts.push(_contents[i].totalAmount / _numOfRewardUnits[i]);
299306
}
300307

301-
require(sumOfRewardUnits % amountPerOpen == 0, "!Amounts");
308+
require(sumOfRewardUnits % amountPerOpen == 0, "!Amt");
302309
supplyToMint = sumOfRewardUnits / amountPerOpen;
303310

304311
if (isUpdate) {

‎docs/Pack.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ Checks whether an account has a particular role; role restricti
464464
### initialize
465465

466466
```solidity
467-
function initialize(address _defaultAdmin, string _name, string _symbol, string _contractURI, address _royaltyRecipient, uint256 _royaltyBps) external nonpayable
467+
function initialize(address _defaultAdmin, string _name, string _symbol, string _contractURI, address[] _trustedForwarders, address _royaltyRecipient, uint256 _royaltyBps) external nonpayable
468468
```
469469

470470

@@ -479,6 +479,7 @@ function initialize(address _defaultAdmin, string _name, string _symbol, string
479479
| _name | string | undefined |
480480
| _symbol | string | undefined |
481481
| _contractURI | string | undefined |
482+
| _trustedForwarders | address[] | undefined |
482483
| _royaltyRecipient | address | undefined |
483484
| _royaltyBps | uint256 | undefined |
484485

‎lib/forge-std‎

‎src/test/Multiwrap.t.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ contract MultiwrapTest is BaseTest {
508508
address recipient = address(0x123);
509509

510510
vm.prank(address(tokenOwner));
511-
vm.expectRevert("no tokens to bind");
511+
vm.expectRevert("!Tokens");
512512
multiwrap.wrap(emptyContent, uriForWrappedToken, recipient);
513513
}
514514

‎src/test/Pack.t.sol‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,11 @@ contract PackTest is BaseTest {
200200
pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, recipient);
201201
}
202202

203+
function test_checkForwarders() public {
204+
assertTrue(pack.isTrustedForwarder(eoaForwarder));
205+
assertTrue(pack.isTrustedForwarder(forwarder));
206+
}
207+
203208
/*///////////////////////////////////////////////////////////////
204209
Unit tests: `createPack`
205210
//////////////////////////////////////////////////////////////*/
@@ -581,8 +586,9 @@ contract PackTest is BaseTest {
581586

582587
address recipient = address(0x123);
583588

589+
bytes memory err = "!Len";
584590
vm.startPrank(address(tokenOwner));
585-
vm.expectRevert("!Contents");
591+
vm.expectRevert(err);
586592
pack.createPack(emptyContent, rewardUnits, packUri, 0, 1, recipient);
587593
}
588594

@@ -594,8 +600,9 @@ contract PackTest is BaseTest {
594600

595601
address recipient = address(0x123);
596602

603+
bytes memory err = "!Len";
597604
vm.startPrank(address(tokenOwner));
598-
vm.expectRevert("!Rewards");
605+
vm.expectRevert(err);
599606
pack.createPack(packContents, rewardUnits, packUri, 0, 1, recipient);
600607
}
601608

@@ -760,7 +767,8 @@ contract PackTest is BaseTest {
760767

761768
address randomRecipient = address(0x12345);
762769

763-
vm.expectRevert("!Recipient");
770+
bytes memory err = "!Bal";
771+
vm.expectRevert(err);
764772
vm.prank(address(tokenOwner));
765773
pack.addPackContents(packId, additionalContents, additionalContentsRewardUnits, randomRecipient);
766774
}
@@ -914,8 +922,9 @@ contract PackTest is BaseTest {
914922
vm.prank(address(tokenOwner));
915923
(, uint256 totalSupply) = pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
916924

925+
bytes memory err = "!Bal";
917926
vm.startPrank(recipient, recipient);
918-
vm.expectRevert("!Balance");
927+
vm.expectRevert(err);
919928
pack.openPack(packId, totalSupply + 1);
920929
}
921930

@@ -942,8 +951,9 @@ contract PackTest is BaseTest {
942951
vm.prank(address(tokenOwner));
943952
pack.createPack(packContents, numOfRewardUnits, packUri, 0, 1, recipient);
944953

954+
bytes memory err = "!Bal";
945955
vm.startPrank(recipient, recipient);
946-
vm.expectRevert("!Balance");
956+
vm.expectRevert(err);
947957
pack.openPack(2, 1);
948958
}
949959

‎src/test/sdk/extension/TokenBundle.t.sol‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,14 @@ contract ExtensionTokenBundle is DSTest, Test {
107107
function test_revert_createBundle_emptyBundle() public {
108108
ITokenBundle.Token[] memory emptyBundle;
109109

110-
vm.expectRevert("no tokens to bind");
110+
vm.expectRevert("!Tokens");
111111
ext.createBundle(emptyBundle, 0);
112112
}
113113

114114
function test_revert_createBundle_existingBundleId() public {
115115
ext.createBundle(bundleContent, 0);
116116

117-
vm.expectRevert("existent at bundleId");
117+
vm.expectRevert("id exists");
118118
ext.createBundle(bundleContent, 0);
119119
}
120120

@@ -246,7 +246,7 @@ contract ExtensionTokenBundle is DSTest, Test {
246246
ext.createBundle(bundleContent, 0);
247247

248248
ITokenBundle.Token[] memory emptyBundle;
249-
vm.expectRevert("no tokens to bind");
249+
vm.expectRevert("!Tokens");
250250
ext.updateBundle(emptyBundle, 0);
251251
}
252252

‎src/test/utils/BaseTest.sol‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import "../mocks/MockERC20.sol";
1010
import "../mocks/MockERC721.sol";
1111
import "../mocks/MockERC1155.sol";
1212
import "contracts/forwarder/Forwarder.sol";
13+
import { ForwarderEOAOnly } from "contracts/forwarder/ForwarderEOAOnly.sol";
1314
import "contracts/TWRegistry.sol";
1415
import "contracts/TWFactory.sol";
1516
import { Multiwrap } from "contracts/multiwrap/Multiwrap.sol";
@@ -41,6 +42,7 @@ abstract contract BaseTest is DSTest, Test {
4142
WETH9 public weth;
4243

4344
address public forwarder;
45+
address public eoaForwarder;
4446
address public registry;
4547
address public factory;
4648
address public fee;
@@ -71,6 +73,7 @@ abstract contract BaseTest is DSTest, Test {
7173
erc1155 = new MockERC1155();
7274
weth = new WETH9();
7375
forwarder = address(new Forwarder());
76+
eoaForwarder = address(new ForwarderEOAOnly());
7477
registry = address(new TWRegistry(forwarder));
7578
factory = address(new TWFactory(forwarder, registry));
7679
contractPublisher = address(new ContractPublisher(forwarder, new MockContractPublisher()));
@@ -92,7 +95,7 @@ abstract contract BaseTest is DSTest, Test {
9295
TWFactory(factory).addImplementation(address(new Split()));
9396
TWFactory(factory).addImplementation(address(new Multiwrap(address(weth))));
9497
TWFactory(factory).addImplementation(address(new MockContract(bytes32("Pack"), 1)));
95-
TWFactory(factory).addImplementation(address(new Pack(address(weth), forwarder)));
98+
TWFactory(factory).addImplementation(address(new Pack(address(weth), eoaForwarder)));
9699
TWFactory(factory).addImplementation(address(new VoteERC20()));
97100
vm.stopPrank();
98101

@@ -226,7 +229,10 @@ abstract contract BaseTest is DSTest, Test {
226229
);
227230
deployContractProxy(
228231
"Pack",
229-
abi.encodeCall(Pack.initialize, (deployer, NAME, SYMBOL, CONTRACT_URI, royaltyRecipient, royaltyBps))
232+
abi.encodeCall(
233+
Pack.initialize,
234+
(deployer, NAME, SYMBOL, CONTRACT_URI, forwarders(), royaltyRecipient, royaltyBps)
235+
)
230236
);
231237
}
232238

0 commit comments

Comments
 (0)