1. implementation of EIP-3525 feature: For any approving function, th…

…e caller MUST be the owner or has been approved with a higher level of authority. 2. fixed bugs of transpile.sh
solv-finance · Mar 1, 2023 · af107ee · af107ee
1 parent 7319f4b
commit af107ee
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 76 deletions.
diff --git a/contracts/ERC3525.sol b/contracts/ERC3525.sol
@@ -152,10 +152,7 @@ contract ERC3525 is Context, IERC3525Metadata, IERC721Enumerable {
         address owner = ERC3525.ownerOf(tokenId_);
         require(to_ != owner, "ERC3525: approval to current owner");
 
-        require(
-            _msgSender() == owner || ERC3525.isApprovedForAll(owner, _msgSender()),
-            "ERC3525: approve caller is not owner nor approved for all"
-        );
+        require(_isApprovedOrOwner(_msgSender(), tokenId_), "ERC3525: approve caller is not owner nor approved");
 
         _approveValue(tokenId_, to_, value_);
     }

diff --git a/contracts/ERC3525SlotApprovable.sol b/contracts/ERC3525SlotApprovable.sol
@@ -53,21 +53,6 @@ contract ERC3525SlotApprovable is Context, ERC3525SlotEnumerable, IERC3525SlotAp
         _approve(to_, tokenId_);
     }
 
-    function approve(uint256 tokenId_, address to_, uint256 value_) public payable virtual override(IERC3525, ERC3525) {
-        address owner = ERC3525.ownerOf(tokenId_);
-        uint256 slot = ERC3525.slotOf(tokenId_);
-        require(to_ != owner, "ERC3525: approval to current owner");
-
-        require(
-            _msgSender() == owner || 
-            ERC3525.isApprovedForAll(owner, _msgSender()) ||
-            ERC3525SlotApprovable.isApprovedForSlot(owner, slot, _msgSender()),
-            "ERC3525: approve caller is not owner nor approved for all/slot"
-        );
-
-        _approveValue(tokenId_, to_, value_);
-    }
-
     function _setApprovalForSlot(
         address owner_,
         uint256 slot_,

diff --git a/contracts/ERC3525SlotApprovableUpgradeable.sol b/contracts/ERC3525SlotApprovableUpgradeable.sol
@@ -59,21 +59,6 @@ contract ERC3525SlotApprovableUpgradeable is Initializable, ContextUpgradeable,
         _approve(to_, tokenId_);
     }
 
-    function approve(uint256 tokenId_, address to_, uint256 value_) public payable virtual override(IERC3525Upgradeable, ERC3525Upgradeable) {
-        address owner = ERC3525Upgradeable.ownerOf(tokenId_);
-        uint256 slot = ERC3525Upgradeable.slotOf(tokenId_);
-        require(to_ != owner, "ERC3525: approval to current owner");
-
-        require(
-            _msgSender() == owner || 
-            ERC3525Upgradeable.isApprovedForAll(owner, _msgSender()) ||
-            ERC3525SlotApprovableUpgradeable.isApprovedForSlot(owner, slot, _msgSender()),
-            "ERC3525: approve caller is not owner nor approved for all/slot"
-        );
-
-        _approveValue(tokenId_, to_, value_);
-    }
-
     function _setApprovalForSlot(
         address owner_,
         uint256 slot_,

diff --git a/contracts/ERC3525Upgradeable.sol b/contracts/ERC3525Upgradeable.sol
@@ -157,10 +157,7 @@ contract ERC3525Upgradeable is Initializable, ContextUpgradeable, IERC3525Metada
         address owner = ERC3525Upgradeable.ownerOf(tokenId_);
         require(to_ != owner, "ERC3525: approval to current owner");
 
-        require(
-            _msgSender() == owner || ERC3525Upgradeable.isApprovedForAll(owner, _msgSender()),
-            "ERC3525: approve caller is not owner nor approved for all"
-        );
+        require(_isApprovedOrOwner(_msgSender(), tokenId_), "ERC3525: approve caller is not owner nor approved");
 
         _approveValue(tokenId_, to_, value_);
     }

diff --git a/contracts/mocks/WithInit.sol → ...ks/ERC3525BaseMockUpgradeableWithInit.sol b/contracts/mocks/WithInit.sol → ...ks/ERC3525BaseMockUpgradeableWithInit.sol
@@ -2,25 +2,47 @@
 pragma solidity >=0.7 <0.9;
 pragma experimental ABIEncoderV2;
 
+import "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";
+
+contract ContextUpgradeableWithInit is ContextUpgradeable {
+    constructor() payable initializer {
+        __Context_init();
+    }
+}
+import "@openzeppelin/contracts-upgradeable/utils/introspection/ERC165Upgradeable.sol";
+
+contract ERC165UpgradeableWithInit is ERC165Upgradeable {
+    constructor() payable initializer {
+        __ERC165_init();
+    }
+}
 import "../ERC3525Upgradeable.sol";
 
 contract ERC3525UpgradeableWithInit is ERC3525Upgradeable {
     constructor(string memory name_, string memory symbol_, uint8 decimals_) payable initializer {
         __ERC3525_init(name_, symbol_, decimals_);
     }
 }
-import "./ERC3525BaseMockUpgradeable.sol";
+import "../ERC3525BurnableUpgradeable.sol";
 
-contract ERC3525BaseMockUpgradeableWithInit is ERC3525BaseMockUpgradeable {
-    constructor(string memory name_, string memory symbol_, uint8 decimals_) payable initializer {
-        __ERC3525BaseMock_init(name_, symbol_, decimals_);
+contract ERC3525BurnableUpgradeableWithInit is ERC3525BurnableUpgradeable {
+    constructor(
+        string memory name_,
+        string memory symbol_,
+        uint8 decimals_
+    ) payable initializer {
+        __ERC3525Burnable_init(name_, symbol_, decimals_);
     }
 }
-import "./ERC3525AllRoundMockUpgradeable.sol";
+import "../ERC3525MintableUpgradeable.sol";
 
-contract ERC3525AllRoundMockUpgradeableWithInit is ERC3525AllRoundMockUpgradeable {
-    constructor(string memory name_, string memory symbol_, uint8 decimals_) payable initializer {
-        __ERC3525AllRoundMock_init(name_, symbol_, decimals_);
+contract ERC3525MintableUpgradeableWithInit is ERC3525MintableUpgradeable {
+    constructor(
+        string memory name_,
+        string memory symbol_,
+        uint8 decimals_
+    ) payable initializer {
+        __ERC3525Mintable_init(name_, symbol_, decimals_);
     }
 }
 import "../ERC3525SlotApprovableUpgradeable.sol";
@@ -41,32 +63,24 @@ contract ERC3525SlotEnumerableUpgradeableWithInit is ERC3525SlotEnumerableUpgrad
         __ERC3525SlotEnumerable_init(name_, symbol_, decimals_);
     }
 }
-import "../periphery/ERC3525MetadataDescriptorUpgradeable.sol";
+import "./ERC3525AllRoundMockUpgradeable.sol";
 
-contract ERC3525MetadataDescriptorUpgradeableWithInit is ERC3525MetadataDescriptorUpgradeable {
-    constructor() payable initializer {
-        __ERC3525MetadataDescriptor_init();
+contract ERC3525AllRoundMockUpgradeableWithInit is ERC3525AllRoundMockUpgradeable {
+    constructor(string memory name_, string memory symbol_, uint8 decimals_) payable initializer {
+        __ERC3525AllRoundMock_init(name_, symbol_, decimals_);
     }
 }
-import "../ERC3525MintableUpgradeable.sol";
+import "./ERC3525BaseMockUpgradeable.sol";
 
-contract ERC3525MintableUpgradeableWithInit is ERC3525MintableUpgradeable {
-    constructor(
-        string memory name_,
-        string memory symbol_,
-        uint8 decimals_
-    ) payable initializer {
-        __ERC3525Mintable_init(name_, symbol_, decimals_);
+contract ERC3525BaseMockUpgradeableWithInit is ERC3525BaseMockUpgradeable {
+    constructor(string memory name_, string memory symbol_, uint8 decimals_) payable initializer {
+        __ERC3525BaseMock_init(name_, symbol_, decimals_);
     }
 }
-import "../ERC3525BurnableUpgradeable.sol";
+import "../periphery/ERC3525MetadataDescriptorUpgradeable.sol";
 
-contract ERC3525BurnableUpgradeableWithInit is ERC3525BurnableUpgradeable {
-    constructor(
-        string memory name_,
-        string memory symbol_,
-        uint8 decimals_
-    ) payable initializer {
-        __ERC3525Burnable_init(name_, symbol_, decimals_);
+contract ERC3525MetadataDescriptorUpgradeableWithInit is ERC3525MetadataDescriptorUpgradeable {
+    constructor() payable initializer {
+        __ERC3525MetadataDescriptor_init();
     }
 }
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@solvprotocol/erc-3525",
-  "version": "1.1.1",
+  "version": "1.2.0",
   "description": "ERC-3525 Reference Implementation for Solidity",
   "license": "MIT",
   "repository": {

diff --git a/scripts/clear-upgradeable.sh b/scripts/clear-upgradeable.sh
@@ -4,5 +4,5 @@ cd contracts
 rm -f *Upgradeable.sol
 find . -name *Upgradeable.sol | xargs rm -f 
 rm -f Initializable.sol
-rm -f mocks/WithInit.sol
+rm -f mocks/ERC3525BaseMockUpgradeableWithInit.sol
 cd ..
diff --git a/scripts/transpile.sh b/scripts/transpile.sh
@@ -29,3 +29,5 @@ rm -rf @openzeppelin
 rm -f contracts/Initializable.sol
 
 node ./scripts/migrate-imports.js
+sed 's/\.\.\/\.\.\/@openzeppelin\/contracts/@openzeppelin\/contracts-upgradeable/g' contracts/mocks/WithInit.sol > contracts/mocks/ERC3525BaseMockUpgradeableWithInit.sol
+rm -f contracts/mocks/WithInit.sol
diff --git a/test/ERC3525.behavior.js b/test/ERC3525.behavior.js
@@ -655,16 +655,15 @@ function shouldBehaveLikeERC3525 (errorPrefix) {
         it('reverts', async function () {
           await expect(
             this.token.connect(other)['approve(uint256,address,uint256)'](firstTokenId, valueApproved.address, this.allowance)
-          ).to.revertedWith('ERC3525: approve caller is not owner nor approved for all');
+          ).to.revertedWith('ERC3525: approve caller is not owner nor approved');
         });
       });
 
       context('when the sender is approved for the given token ID', function () {
-        it('reverts', async function () {
+        it('approve value by token ID approved address', async function () {
           await this.token.connect(firstOwner)['approve(address,uint256)'](approved.address, firstTokenId);
-          await expect(
-            this.token.connect(approved)['approve(uint256,address,uint256)'](firstTokenId, valueApproved.address, this.allowance)
-          ).to.revertedWith('ERC3525: approve caller is not owner nor approved for all');
+          await this.token.connect(approved)['approve(uint256,address,uint256)'](firstTokenId, valueApproved.address, this.allowance);
+          expect(await this.token.allowance(firstTokenId, valueApproved.address)).to.be.equal(this.allowance);
         });
       });
 
@@ -1119,13 +1118,13 @@ function shouldBehaveLikeERC3525SlotApprovable (errorPrefix) {
           it('reverts when approving others values in the same slot', async function () {
             await expect(
               this.token.connect(slotOperator)['approve(uint256,address,uint256)'](secondTokenId, valueApproved.address, 10)
-            ).to.revertedWith('ERC3525: approve caller is not owner nor approved for all/slot');
+            ).to.revertedWith('ERC3525: approve caller is not owner nor approved');
           });
 
           it('reverts when approving owners values in other slot', async function () {
             await expect(
               this.token.connect(slotOperator)['approve(uint256,address,uint256)'](thirdTokenId, valueApproved.address, 10)
-            ).to.revertedWith('ERC3525: approve caller is not owner nor approved for all/slot');
+            ).to.revertedWith('ERC3525: approve caller is not owner nor approved');
           });
         });
 
@@ -1208,7 +1207,7 @@ function shouldBehaveLikeERC3525SlotApprovable (errorPrefix) {
         it('reverts when approving values of the owner in the unapproved slot', async function () {
           await expect(
             this.token.connect(slotOperator)['approve(uint256,address,uint256)'](firstTokenId, approved.address, 100)
-          ).to.revertedWith('ERC3525: approve caller is not owner nor approved for all/slot');
+          ).to.revertedWith('ERC3525: approve caller is not owner nor approved');
         });
 
         it('reverts when transferring tokens of the owner in the unapproved slot', async function () {