ditto-lab / ditto

NFT Future protocol
MIT License
60 stars 3 forks source link

Handle receiver revert on NFT trade #86

Closed 0xbok closed 1 year ago

0xbok commented 1 year ago

PR to get early feedback. Current flow (we can change it as needed):

Bug fix: pass correct index in return data on nft receiver function in dittomachine.

0xbok commented 1 year ago

I dont see any negative consequence due to this. Do you have any concern?

On Sat, Nov 26, 2022 at 2:50 AM calvbore @.***> wrote:

@.**** commented on this pull request.

In src/DittoMachine.sol https://github.com/ditto-lab/ditto/pull/86#discussion_r1032689426:

  • function bumpSubsidy(address nft, uint id, address ERC20Contract, bool floor, uint128 amount) external {
  • uint protoId = uint(keccak256(abi.encodePacked(
  • nft,
  • id,
  • ERC20Contract,
  • false
  • )));
  • uint cloneId = protoIdToIndexHead[protoId];
  • assembly ("memory-safe") {
  • mstore(0, protoId)
  • mstore(0x20, cloneId)
  • cloneId := keccak256(0, 0x40)
  • }
  • SafeTransferLib.safeTransferFrom(
  • ERC20(ERC20Contract),
  • msg.sender,
  • address(this),
  • amount
  • );
  • cloneIdToSubsidy[cloneId] += amount;
  • }

Just something to consider here, but do we want to allow anyone to increase the subsidy of a clone?

— Reply to this email directly, view it on GitHub https://github.com/ditto-lab/ditto/pull/86#pullrequestreview-1194773628, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAM4POYKA7H7WHAZUCZIKHDWKEUSPANCNFSM6AAAAAARQ5JGYM . You are receiving this because you authored the thread.Message ID: @.***>

calvbore commented 1 year ago

@0xbok lgtm