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 review - removes all NFT related code #114

Merged
merged 33 commits into from
Feb 28, 2023
Merged

Fix review - removes all NFT related code #114

merged 33 commits into from
Feb 28, 2023

Conversation

FlacoJones
Copy link
Contributor

Removes the following methods from AtomicBountyV1.sol:

  • receiveNft

Removes the following storage variables from BountyStorageCore.sol:

  • nftDepositLimit
  • isNFT

Removes the following methods from BountyCore.sol:

  • claimNft
  • _receiveNft
  • getNftDeposits

Removes the following methods from TieredBountyCore.sol:

  • receiveNft

Removes the following methods from DepositManagerV1.sol:

  • fundBountyNFT

  • Modifies the ClaimManagerV1::claimBounty and ClaimManagerV1::permissionedClaimTieredBounty to remove the logic that looped over NFT deposits

  • Removes ERC721Receiver.sol import in BountyStorageCore.sol

Removes if/else logic in BountyCore that used to check whether or not the funding is with an NFT

  • updates all tests to remove nft related tests

  • updates deploy scripts to deploy without MockNFT.sol

NOTE: I keep the tokenId parameter in bounty funding, and also keep the NFT related events though there is no way no to emit them. This is because we don't want to have to make backwards incompatible changes to the current ABIs used across many services

…g the Ongoing+TieredPercentage implementations, interfaces, and methods in OpenQV1 and ClaimManagerV1
…DDRESS_LIMIT, simply checks if token is whitelisted or not
@FlacoJones FlacoJones changed the title Fix review nfts Fix review - removes all NFT related code Feb 25, 2023
Copy link

@IAm0x52 IAm0x52 left a comment

Choose a reason for hiding this comment

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

Changes look good. Removes all logic from bounties. Seems to have gotten all related code. Will keep my out for any that was missed on when checking the rest of the PRs

@FlacoJones FlacoJones merged commit dc6fc17 into audit Feb 28, 2023
@FlacoJones FlacoJones deleted the fix-review-nfts branch August 10, 2023 02:30
@FlacoJones FlacoJones restored the fix-review-nfts branch August 10, 2023 02:30
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.

2 participants