Prevent silent mint if the token exists already (#928)

* Unit tests - price discovery offer * Prevent silent mint of existing tokens * Fix failing unit test * Enable "rewrapping"
bosonprotocol · Apr 8, 2024 · a0a2aee · a0a2aee
1 parent b36d63b
commit a0a2aee
Show file tree

Hide file tree

Showing 3 changed files with 336 additions and 176 deletions.
diff --git a/contracts/protocol/clients/voucher/BosonVoucher.sol b/contracts/protocol/clients/voucher/BosonVoucher.sol
@@ -388,12 +388,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable
         bool committable = isTokenCommittable(_tokenId);
 
         if (committable) {
-            if (_from == address(this) || _from == owner()) {
-                // If offer is committable, temporarily update _owners, so transfer succeeds
-                silentMint(_from, _tokenId);
-            }
-
-            _isCommittable = true;
+            silentMint(_from, _tokenId);
         }
 
         super.transferFrom(_from, _to, _tokenId);
@@ -411,12 +406,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable
         bool committable = isTokenCommittable(_tokenId);
 
         if (committable) {
-            if (_from == address(this) || _from == owner()) {
-                // If offer is committable, temporarily update _owners, so transfer succeeds
-                silentMint(_from, _tokenId);
-            }
-
-            _isCommittable = true;
+            silentMint(_from, _tokenId);
         }
 
         super.safeTransferFrom(_from, _to, _tokenId, _data);
@@ -776,10 +766,12 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable
      * Updates owners, but do not emit Transfer event. Event was already emited during pre-mint.
      */
     function silentMint(address _from, uint256 _tokenId) internal {
-        if (_from != owner() && _from != address(this)) revert NoSilentMintAllowed();
+        if (!_exists(_tokenId) && (_from == address(this) || _from == owner())) {
+            // update data, so transfer will succeed
+            getERC721UpgradeableStorage()._owners[_tokenId] = _from;
+        }
 
-        // update data, so transfer will succeed
-        getERC721UpgradeableStorage()._owners[_tokenId] = _from;
+        _isCommittable = true;
     }
 
     /*

diff --git a/test/protocol/PriceDiscoveryHandlerFacet.js b/test/protocol/PriceDiscoveryHandlerFacet.js
@@ -1414,6 +1414,8 @@ describe("IPriceDiscoveryHandlerFacet", function () {
                 priceDiscovery = new PriceDiscovery(price, Side.Wrapper, wethAddress, wethAddress, calldata);
 
                 // Transfer the voucher to weth to pass the "is owner" check
+                // Use token that was not wrapped yet
+                tokenId = deriveTokenId(offer.id, 3);
                 await bosonVoucher.connect(assistant).transferFrom(assistant.address, wethAddress, tokenId);
 
                 // Attempt to commit, expecting revert