Fix Allowances (#24)

* add custom NegRiskCtfExchange

* NegRiskAdapter_ERC1155Operations_Test

* add comment

* add first fillOrder test

* NegRiskCtfExchange tests

* update interface

* remove abis

* cleanup

* fix secondary approvals

* cleanup

* add withdrawFees test

* add additional safeTransferFrom test

* reverse return/revert

* remove commented code

* update safeTransferFrom, remove safeBatchTransferFrom

* clean up and add natsec

* small updates

* change DELAY_PERIOD to 1 hour

* remove commented tests + extra NegRiskFeeModule approval

* update natspec

.forge-snapshots/ConditionalTokens_mergePositions.snap

@@ -1 +1 @@
-52410
+67211

.forge-snapshots/ConditionalTokens_splitPosition.snap

@@ -1 +1 @@
-99336
+114137

.forge-snapshots/NegRiskAdapter_convertPositions_32.snap

@@ -1 +1 @@
-4626380
+4522762

.forge-snapshots/NegRiskAdapter_convertPositions_5.snap

@@ -1 +1 @@
-687233
+717003

.forge-snapshots/NegRiskAdapter_convertPositions_64.snap

@@ -1 +1 @@
-9540525
+9331964

.forge-snapshots/NegRiskAdapter_mergePositions.snap

@@ -1 +1 @@
-191072
+220694

.forge-snapshots/NegRiskAdapter_prepareMarket.snap

@@ -1 +1 @@
-33098
+33110

.forge-snapshots/NegRiskAdapter_prepareQuestion.snap

@@ -1 +1 @@
-37479
+37634

.forge-snapshots/NegRiskAdapter_splitPosition.snap

@@ -1 +1 @@
-215878
+245589

.forge-snapshots/Vault_transferERC1155.snap

@@ -1 +1 @@
-32809
+34921

.forge-snapshots/Vault_transferERC20.snap

@@ -1 +1 @@
-31009
+33121

.forge-snapshots/WrappedCollateral_unwrap.snap

@@ -1 +1 @@
-28664
+30764

.forge-snapshots/WrappedCollateral_wrap.snap

@@ -1 +1 @@
-75728
+77828

.gitignore

@@ -15,3 +15,5 @@ docs/
 
 # python virtualenv
 env/
+
+node_modules/

.gitmodules

@@ -7,3 +7,9 @@
 [submodule "lib/forge-gas-snapshot"]
 	path = lib/forge-gas-snapshot
 	url = https://github.com/marktoda/forge-gas-snapshot
+[submodule "lib/ctf-exchange"]
+	path = lib/ctf-exchange
+	url = https://github.com/Polymarket/ctf-exchange
+[submodule "lib/exchange-fee-module"]
+	path = lib/exchange-fee-module
+	url = https://github.com/Polymarket/exchange-fee-module

.vscode/settings.json

@@ -3,8 +3,8 @@
   "solidity.packageDefaultDependenciesDirectory": "lib",
   "solidity.compileUsingRemoteVersion": "v0.8.19+commit.7dd6d404",
   "editor.formatOnSave": true,
-  "[solidity]": {
-    "editor.defaultFormatter": "JuanBlanco.solidity"
-  },
+  // "[solidity]": {
+  //   "editor.defaultFormatter": "JuanBlanco.solidity"
+  // },
   "solidity.formatter": "forge"
 }

foundry.toml

@@ -1,11 +1,10 @@
 [profile.default]
-solc = "0.8.19"
 src = "src"
 out = "out"
 libs = ["lib"]
 
 ffi = true
-fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"},{ access = "read", path = "./artifacts/"}]
+fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"},{ access = "read", path = "./artifacts/"}, { access = "read", path = "./out/"}]
 
 [fmt]
 line_length = 120

remappings.txt

@@ -2,3 +2,5 @@ ds-test/=lib/forge-std/lib/ds-test/src/
 forge-std/=lib/forge-std/src/
 solmate/=lib/solmate/src/
 forge-gas-snapshot/=lib/forge-gas-snapshot/src/
+
+openzeppelin-contracts/=lib/ctf-exchange/lib/openzeppelin-contracts/contracts/

src/NegRiskAdapter.sol

@@ -4,21 +4,23 @@ pragma solidity 0.8.19;
 import {ERC1155TokenReceiver} from "lib/solmate/src/tokens/ERC1155.sol";
 import {ERC20} from "lib/solmate/src/tokens/ERC20.sol";
 import {SafeTransferLib} from "lib/solmate/src/utils/SafeTransferLib.sol";
-
 import {WrappedCollateral} from "src/WrappedCollateral.sol";
 import {MarketData, MarketStateManager, IMarketStateManagerEE} from "src/modules/MarketDataManager.sol";
 import {CTHelpers} from "src/libraries/CTHelpers.sol";
 import {Helpers} from "src/libraries/Helpers.sol";
 import {NegRiskIdLib} from "src/libraries/NegRiskIdLib.sol";
 import {IConditionalTokens} from "src/interfaces/IConditionalTokens.sol";
+import {Auth} from "src/modules/Auth.sol";
+import {IAuthEE} from "src/modules/interfaces/IAuth.sol";
 
 /// @title INegRiskAdapterEE
 /// @notice NegRiskAdapter Errors and Events
-interface INegRiskAdapterEE is IMarketStateManagerEE {
+interface INegRiskAdapterEE is IMarketStateManagerEE, IAuthEE {
     error InvalidIndexSet();
     error LengthMismatch();
     error UnexpectedCollateralToken();
     error NoConvertiblePositions();
+    error NotApprovedForAll();
 
     event MarketPrepared(bytes32 indexed marketId, address indexed oracle, uint256 feeBips, bytes data);
     event QuestionPrepared(bytes32 indexed marketId, bytes32 indexed questionId, uint256 index, bytes data);
@@ -37,7 +39,7 @@ interface INegRiskAdapterEE is IMarketStateManagerEE {
 /// @notice And the adapter allows for the conversion of a set of no positions, to collateral plus the set of
 /// complementary yes positions
 /// @author Mike Shrieve ([email protected])
-contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAdapterEE {
+contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAdapterEE, Auth {
     using SafeTransferLib for ERC20;
 
     /*//////////////////////////////////////////////////////////////
@@ -56,7 +58,7 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
                               CONSTRUCTOR
     //////////////////////////////////////////////////////////////*/
 
-    /// @param _ctf  - ConditionalTokens address
+    /// @param _ctf        - ConditionalTokens address
     /// @param _collateral - collateral address
     constructor(address _ctf, address _collateral, address _vault) {
         ctf = IConditionalTokens(_ctf);
@@ -118,7 +120,7 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
 
     /// @notice Splits collateral to a complete set of conditional tokens for a single question
     /// @param _conditionId - the conditionId for the question
-    /// @param _amount - the amount of collateral to split
+    /// @param _amount      - the amount of collateral to split
     function splitPosition(bytes32 _conditionId, uint256 _amount) public {
         col.safeTransferFrom(msg.sender, address(this), _amount);
         wcol.wrap(address(this), _amount);
@@ -137,8 +139,8 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
     /// @notice Merges a complete set of conditional tokens for a single question to collateral
     /// @notice This function signature is the same as the CTF's mergePositions
     /// @param _collateralToken - the collateral token, must be the same as the adapter's collateral token
-    /// @param _conditionId - the conditionId for the question
-    /// @param _amount - the amount of collateral to merge
+    /// @param _conditionId     - the conditionId for the question
+    /// @param _amount          - the amount of collateral to merge
     function mergePositions(
         address _collateralToken,
         bytes32,
@@ -152,7 +154,7 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
 
     /// @notice Merges a complete set of conditional tokens for a single question to collateral
     /// @param _conditionId - the conditionId for the question
-    /// @param _amount - the amount of collateral to merge
+    /// @param _amount      - the amount of collateral to merge
     function mergePositions(bytes32 _conditionId, uint256 _amount) public {
         uint256[] memory positionIds = Helpers.positionIds(address(wcol), _conditionId);
 
@@ -164,13 +166,53 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
         emit PositionsMerge(msg.sender, _conditionId, _amount);
     }
 
+    /*//////////////////////////////////////////////////////////////
+                           ERC1155 OPERATIONS
+    //////////////////////////////////////////////////////////////*/
+
+    /// @notice Proxies ERC1155 balanceOf to the CTF
+    /// @param _owner   - the owner of the tokens
+    /// @param _id      - the positionId
+    /// @return balance - the owner's balance
+    function balanceOf(address _owner, uint256 _id) external view returns (uint256) {
+        return ctf.balanceOf(_owner, _id);
+    }
+
+    /// @notice Proxies ERC1155 balanceOfBatch to the CTF
+    /// @param _owners   - the owners of the tokens
+    /// @param _ids      - the positionIds
+    /// @return balances - the owners' balances
+    function balanceOfBatch(address[] memory _owners, uint256[] memory _ids) external view returns (uint256[] memory) {
+        return ctf.balanceOfBatch(_owners, _ids);
+    }
+
+    /// @notice Proxies ERC1155 safeTransferFrom to the CTF
+    /// @notice Can only be called by an admin
+    /// @notice Requires this contract to be approved for all
+    /// @notice Requires the sender to be approved for all
+    /// @param _from  - the owner of the tokens
+    /// @param _to    - the recipient of the tokens
+    /// @param _id    - the positionId
+    /// @param _value - the amount of tokens to transfer
+    /// @param _data  - the data to pass to the recipient
+    function safeTransferFrom(address _from, address _to, uint256 _id, uint256 _value, bytes calldata _data)
+        external
+        onlyAdmin
+    {
+        if (!ctf.isApprovedForAll(_from, msg.sender)) {
+            revert NotApprovedForAll();
+        }
+
+        return ctf.safeTransferFrom(_from, _to, _id, _value, _data);
+    }
+
     /*//////////////////////////////////////////////////////////////
                             REDEEM POSITION
     //////////////////////////////////////////////////////////////*/
 
     /// @notice Redeem a set of conditional tokens for collateral
     /// @param _conditionId - conditionId of the conditional tokens to redeem
-    /// @param _amounts - amounts of conditional tokens to redeem
+    /// @param _amounts     - amounts of conditional tokens to redeem
     /// _amounts should always have length 2, with the first element being the amount of yes tokens to redeem and the
     /// second element being the amount of no tokens to redeem
     function redeemPositions(bytes32 _conditionId, uint256[] calldata _amounts) public {
@@ -373,7 +415,7 @@ contract NegRiskAdapter is ERC1155TokenReceiver, MarketStateManager, INegRiskAda
                                 INTERNAL
     //////////////////////////////////////////////////////////////*/
 
-    /// @dev internal function to avoid stack to deep in convertPositions
+    /// @dev internal function to avoid stack too deep in convertPositions
     function _splitPosition(bytes32 _conditionId, uint256 _amount) internal {
         ctf.splitPosition(address(wcol), bytes32(0), _conditionId, Helpers.partition(), _amount);
     }

src/NegRiskCtfExchange.sol

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.15;
+
+import {CTFExchange} from "../lib/ctf-exchange/src/exchange/CTFExchange.sol";
+import {IConditionalTokens} from "./interfaces/IConditionalTokens.sol";
+
+contract NegRiskCtfExchange is CTFExchange {
+    constructor(address _collateral, address _ctf, address _negRiskAdapter, address _proxyFactory, address _safeFactory)
+        CTFExchange(_collateral, _negRiskAdapter, _proxyFactory, _safeFactory)
+    {
+        IConditionalTokens(_ctf).setApprovalForAll(_negRiskAdapter, true);
+        IConditionalTokens(_ctf).setApprovalForAll(address(this), true);
+    }
+}

src/NegRiskFeeModule.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.15;
+
+import {FeeModule, IExchange} from "../lib/exchange-fee-module/src/FeeModule.sol";
+import {IConditionalTokens} from "./interfaces/IConditionalTokens.sol";
+
+contract NegRiskFeeModule is FeeModule {
+    constructor(address _negRiskCtfExchange, address _negRiskAdapter, address _ctf) FeeModule(_negRiskCtfExchange) {
+        IConditionalTokens(_ctf).setApprovalForAll(_negRiskAdapter, true);
+        IConditionalTokens(_ctf).setApprovalForAll(address(this), true);
+    }
+}

src/NegRiskOperator.sol

@@ -47,7 +47,7 @@ contract NegRiskOperator is INegRiskOperatorEE, Auth {
 
     NegRiskAdapter public immutable nrAdapter;
     address public oracle;
-    uint256 public constant DELAY_PERIOD = 12 hours;
+    uint256 public constant DELAY_PERIOD = 1 hours;
 
     mapping(bytes32 _requestId => bytes32) public questionIds;
     mapping(bytes32 _questionId => bool) public results;
@@ -91,8 +91,8 @@ contract NegRiskOperator is INegRiskOperatorEE, Auth {
     //////////////////////////////////////////////////////////////*/
 
     /// @notice Prepares a market on the NegRiskAdapter
-    /// @param _feeBips - the market's fee rate out of 10_000
-    /// @param _data - the market metadata to be passed to the NegRiskAdapter
+    /// @param _feeBips  - the market's fee rate out of 10_000
+    /// @param _data     - the market metadata to be passed to the NegRiskAdapter
     /// @return marketId - the market id
     function prepareMarket(uint256 _feeBips, bytes calldata _data) external onlyAdmin returns (bytes32) {
         bytes32 marketId = nrAdapter.prepareMarket(_feeBips, _data);
@@ -107,9 +107,10 @@ contract NegRiskOperator is INegRiskOperatorEE, Auth {
     /// @notice Prepares a question on the NegRiskAdapter
     /// @notice OnlyAdmin
     /// @notice Only one question can be prepared per requestId
-    /// @param _marketId - the id of the market in which to prepare the question
-    /// @param _data - the question metadata to be passed to the NegRiskAdapter
-    /// @param _requestId - the question's oracle request id
+    /// @param _marketId   - the id of the market in which to prepare the question
+    /// @param _data       - the question metadata to be passed to the NegRiskAdapter
+    /// @param _requestId  - the question's oracle request id
+    /// @return questionId - the resulting question id
     function prepareQuestion(bytes32 _marketId, bytes calldata _data, bytes32 _requestId)
         external
         onlyAdmin
@@ -136,7 +137,7 @@ contract NegRiskOperator is INegRiskOperatorEE, Auth {
     /// @notice Only one report can be made per question
     /// @notice Sets the boolean result and reportedAt timestamp for the question
     /// @param _requestId - the question's oracle request id
-    /// @param _payouts - the payouts to be reported, [1,0] if true, [0,1] if false, any other payouts are invalid
+    /// @param _payouts   - the payouts to be reported, [1,0] if true, [0,1] if false, any other payouts are invalid
     function reportPayouts(bytes32 _requestId, uint256[] calldata _payouts) external onlyOracle {
         if (_payouts.length != 2) {
             revert InvalidPayouts();
@@ -211,6 +212,8 @@ contract NegRiskOperator is INegRiskOperatorEE, Auth {
     /// @notice Resolves a flagged question on the NegRiskAdapter
     /// @notice OnlyAdmin
     /// @notice A flagged question can only be resolved if the delay period has passed since the question was flagged
+    /// @param _questionId - the id of the question to be resolved
+    /// @param _result     - the boolean result of the question
     function emergencyResolveQuestion(bytes32 _questionId, bool _result) external onlyAdmin {
         uint256 flaggedAt_ = flaggedAt[_questionId];
 

src/WrappedCollateral.sol

@@ -64,31 +64,31 @@ contract WrappedCollateral is IWrappedCollateralEE, ERC20 {
 
     /// @notice Wraps the specified amount of tokens
     /// @notice Can only be called by the owner
-    /// @param _to The address to send the wrapped tokens to
-    /// @param _amount The amount of tokens to wrap
+    /// @param _to     - the address to send the wrapped tokens to
+    /// @param _amount - the amount of tokens to wrap
     function wrap(address _to, uint256 _amount) external onlyOwner {
         ERC20(underlying).safeTransferFrom(msg.sender, address(this), _amount);
         _mint(_to, _amount);
     }
 
     /// @notice Burns the specified amount of tokens
     /// @notice Can only be called by the owner
-    /// @param _amount The amount of tokens to burn
+    /// @param _amount - the amount of tokens to burn
     function burn(uint256 _amount) external onlyOwner {
         _burn(msg.sender, _amount);
     }
 
     /// @notice Mints the specified amount of tokens
     /// @notice Can only be called by the owner
-    /// @param _amount The amount of tokens to mint
+    /// @param _amount - the amount of tokens to mint
     function mint(uint256 _amount) external onlyOwner {
         _mint(msg.sender, _amount);
     }
 
     /// @notice Releases the specified amount of the underlying token
     /// @notice Can only be called by the owner
-    /// @param _to The address to send the released tokens to
-    /// @param _amount The amount of tokens to release
+    /// @param _to     - the address to send the released tokens to
+    /// @param _amount - the amount of tokens to release
     function release(address _to, uint256 _amount) external onlyOwner {
         ERC20(underlying).safeTransfer(_to, _amount);
     }