Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Allowances #24

Merged
merged 20 commits into from
Nov 27, 2023
Merged

Fix Allowances #24

merged 20 commits into from
Nov 27, 2023

Conversation

mshrieve
Copy link
Contributor

@mshrieve mshrieve commented Nov 1, 2023

This PR addresses issues integrating the (NegRisk)CtfExchange with the NegRiskAdapter.
In matchOrders, it is necessary to call safeTransferFrom, as well to query the balances of an address.

The NegRiskCtfExchange treats the NegRiskAdapter as the CTF, so it is necessary to proxy balanceOf and safeTransferFrom to the CTF.

The primary addition is the following:

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);
  }

There are two safety measures to prevent unauthorized transfers.

  1. safeTransferFrom is marked as onlyAdmin. This means that no caller may call this function unless they are added as an admin on the NegRiskAdapter. There are only two addresses that should call this function: the NegRiskCtfExchange during the execution of matchOrders and the NegRiskFeeModule during the execution of withdrawFees.
  2. safeTransferFrom checks that the _from address has approved msg.sender to transfer their conditional tokens. This means that the _from address must approve the NegRiskCtfAdapter. This also means that the _from address can prevent the NegRiskCtfAdapter from transferring their conditional tokens by revoking allowances.

Additionally, the CTF itself enforces that the _from address must approve the NegRiskAdapter to spend their conditional tokens. If the _from address has not approved the NegRiskAdapter, the call will revert at the CTF.


This PR also changes the NegRiskOperator's DELAY_PERIOD to 1 hour.

Copy link

@JonathanAmenechi JonathanAmenechi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@JonathanAmenechi JonathanAmenechi self-requested a review November 3, 2023 18:43
src/NegRiskAdapter.sol Outdated Show resolved Hide resolved
src/NegRiskAdapter.sol Outdated Show resolved Hide resolved
@L-Kov L-Kov self-requested a review November 23, 2023 04:04
@JonathanAmenechi JonathanAmenechi self-requested a review November 27, 2023 00:30
@mshrieve mshrieve merged commit 926a810 into main Nov 27, 2023
1 check passed
@mshrieve mshrieve deleted the fix/allowances branch November 27, 2023 19:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants