Skip to content

Commit b3544ec

Browse files
authored
Open edition macro audit fixes (#397)
* rename account factory callbacks * Fix [M-1] Native token can get locked in OpenEditionERC721 * Fix [Q-1, Q-2] * pkg update
1 parent e7fd7f7 commit b3544ec

File tree

2 files changed

+11
-9
lines changed

2 files changed

+11
-9
lines changed

contracts/OpenEditionERC721.sol

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import "./extension/PlatformFee.sol";
3232
import "./extension/Royalty.sol";
3333
import "./extension/PrimarySale.sol";
3434
import "./extension/Ownable.sol";
35-
import "./extension/DelayedReveal.sol";
3635
import "./extension/SharedMetadata.sol";
3736
import "./extension/PermissionsEnumerable.sol";
3837
import "./extension/Drop.sol";
@@ -63,7 +62,7 @@ contract OpenEditionERC721 is
6362

6463
/// @dev Only transfers to or from TRANSFER_ROLE holders are valid, when transfers are restricted.
6564
bytes32 private transferRole;
66-
/// @dev Only MINTER_ROLE holders can sign off on `MintRequest`s and lazy mint tokens.
65+
/// @dev Only MINTER_ROLE holders can update the shared metadata of tokens.
6766
bytes32 private minterRole;
6867

6968
/// @dev Max bps in the thirdweb system.
@@ -158,13 +157,16 @@ contract OpenEditionERC721 is
158157
}
159158

160159
uint256 totalPrice = _quantityToClaim * _pricePerToken;
161-
address saleRecipient = _primarySaleRecipient == address(0) ? primarySaleRecipient() : _primarySaleRecipient;
162160

161+
bool validMsgValue;
163162
if (_currency == CurrencyTransferLib.NATIVE_TOKEN) {
164-
if (msg.value != totalPrice) {
165-
revert("!Price");
166-
}
163+
validMsgValue = msg.value == totalPrice;
164+
} else {
165+
validMsgValue = msg.value == 0;
167166
}
167+
require(validMsgValue, "!V");
168+
169+
address saleRecipient = _primarySaleRecipient == address(0) ? primarySaleRecipient() : _primarySaleRecipient;
168170

169171
uint256 fees;
170172
address feeRecipient;
@@ -178,7 +180,7 @@ contract OpenEditionERC721 is
178180
fees = (totalPrice * platformFeeBps) / MAX_BPS;
179181
}
180182

181-
require(totalPrice >= fees, "Price < fees");
183+
require(totalPrice >= fees, "!F");
182184

183185
CurrencyTransferLib.transferCurrency(_currency, _msgSender(), feeRecipient, fees);
184186
CurrencyTransferLib.transferCurrency(_currency, _msgSender(), saleRecipient, totalPrice - fees);
@@ -224,7 +226,7 @@ contract OpenEditionERC721 is
224226
return hasRole(DEFAULT_ADMIN_ROLE, _msgSender());
225227
}
226228

227-
/// @dev Returns whether lazy minting can be done in the given execution context.
229+
/// @dev Returns whether the shared metadata of tokens can be set in the given execution context.
228230
function _canSetSharedMetadata() internal view virtual override returns (bool) {
229231
return hasRole(minterRole, _msgSender());
230232
}

contracts/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@thirdweb-dev/contracts",
33
"description": "Collection of smart contracts deployable via the thirdweb SDK, dashboard and CLI",
4-
"version": "3.5.5",
4+
"version": "3.5.6",
55
"license": "Apache-2.0",
66
"repository": {
77
"type": "git",

0 commit comments

Comments
 (0)