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: emit correct transfer and approval events (#194) #215

Merged
76 changes: 51 additions & 25 deletions README.md

Large diffs are not rendered by default.

15 changes: 7 additions & 8 deletions contracts/HtsSystemContract.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {HederaResponseCodes} from "./HederaResponseCodes.sol";

address constant HTS_ADDRESS = address(0x167);

contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
contract HtsSystemContract is IHederaTokenService {

// All ERC20 properties are accessed with a `delegatecall` from the Token Proxy.
// See `__redirectForToken` for more details.
Expand Down Expand Up @@ -465,7 +465,7 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
address to = address(bytes20(msg.data[72:92]));
uint256 amount = uint256(bytes32(msg.data[92:124]));
_approve(from, to, amount);
emit Approval(from, to, amount);
emit IERC20Events.Approval(from, to, amount);
return abi.encode(true);
}
if (selector == this.approveNFT.selector) {
Expand Down Expand Up @@ -561,7 +561,7 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
uint256 amount = uint256(bytes32(msg.data[60:92]));
address owner = msg.sender;
_approve(owner, spender, amount);
emit Approval(owner, spender, amount);
emit IERC20Events.Approval(owner, spender, amount);
return abi.encode(true);
}
return _redirectForHRC719(selector);
Expand Down Expand Up @@ -735,7 +735,7 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
require(from != address(0), "hts: invalid sender");
require(to != address(0), "hts: invalid receiver");
_update(from, to, amount);
emit Transfer(from, to, amount);
emit IERC20Events.Transfer(from, to, amount);
}

function _transferNFT(address sender, address from, address to, uint256 serialId) private {
Expand Down Expand Up @@ -763,7 +763,7 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {

// Set the new owner
assembly { sstore(slot, to) }
emit Transfer(from, to, serialId);
emit IERC721Events.Transfer(from, to, serialId);
}

function _update(address from, address to, uint256 amount) public {
Expand Down Expand Up @@ -808,8 +808,7 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
bytes32 slot = _getApprovedSlot(uint32(serialId));
address newApproved = isApproved ? spender : address(0);
assembly { sstore(slot, newApproved) }

emit Approval(owner, spender, serialId);
emit IERC721Events.Approval(owner, spender, serialId);
}

/**
Expand All @@ -830,6 +829,6 @@ contract HtsSystemContract is IHederaTokenService, IERC20Events, IERC721Events {
require(operator != address(0) && operator != sender, "setApprovalForAll: invalid operator");
bytes32 slot = _isApprovedForAllSlot(sender, operator);
assembly { sstore(slot, approved) }
emit ApprovalForAll(sender, operator, approved);
emit IERC721Events.ApprovalForAll(sender, operator, approved);
}
}
2 changes: 1 addition & 1 deletion contracts/IERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ interface IERC20 {

/**
* @dev Returns the value of tokens owned by `account`.
*/
*/
function balanceOf(address account) external view returns (uint256);

/**
Expand Down
27 changes: 27 additions & 0 deletions contracts/IERC721.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,34 @@
pragma solidity ^0.8.0;

/**
* These events should be emitted by `transfer|transferFrom`, `approve` and `setApprovalForAll` respectively.
*
* See https://ethereum.org/en/developers/docs/standards/tokens/erc-721/#events for more information.
*/
interface IERC721Events {
/**
* @dev Emitted when ownership of any NFT changes by any mechanism.
* This event emits when NFTs are created (`from` == 0) and destroyed (`to` == 0).
* Otherwise, it indicates that the token with ID {tokenId} was transferred from {from} to {to},
* where {from} represents the previous owner of the token, not the approved spender.
*
* Exception: during contract creation, any number of NFTs may be created and assigned without emitting Transfer.
*
* At the time of any transfer, the approved address for that NFT (if any) is reset to none.
*/
event Transfer(address indexed from, address indexed to, uint256 indexed tokenId);
acuarica marked this conversation as resolved.
Show resolved Hide resolved

/**
* @dev Emitted when the approved address for an NFT is changed or reaffirmed from {from} to {to} address.
* The zero {to} address indicates there will be no approved address.
* Additionally the approved address for that NFT (if any) is reset to none.
*/
event Approval(address indexed owner, address indexed approved, uint256 indexed tokenId);

/**
* @dev Emitted when an operator {operator} is enabled or disabled {approved}
* for an owner {owner}. The operator {operator} can than manage all NFTs of the owner {owner}.
*/
event ApprovalForAll(address indexed owner, address indexed operator, bool approved);
}

Expand Down Expand Up @@ -45,6 +70,8 @@ interface IERC721 {
*
* Requirements:
* - `serialId` must exist.
*
* This value changes when {approve} or {transferFrom} are called.
*/
function ownerOf(uint256 serialId) external view returns (address);

Expand Down
9 changes: 4 additions & 5 deletions test/ERC721.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ pragma solidity ^0.8.0;
import {HtsSystemContract} from "../contracts/HtsSystemContract.sol";
import {Test, console} from "forge-std/Test.sol";
import {IERC721, IERC721Events} from "../contracts/IERC721.sol";
import {IERC20Events} from "../contracts/IERC20.sol";
import {TestSetup} from "./lib/TestSetup.sol";

contract ERC721TokenTest is Test, TestSetup, IERC721Events, IERC20Events {
contract ERC721TokenTest is Test, TestSetup {
arianejasuwienas marked this conversation as resolved.
Show resolved Hide resolved

function setUp() external {
setUpMockStorageForNonFork();
Expand Down Expand Up @@ -39,7 +38,7 @@ contract ERC721TokenTest is Test, TestSetup, IERC721Events, IERC20Events {
uint256 tokenId = 1;
vm.startPrank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit Transfer(CFNFTFF_TREASURY, to, tokenId);
emit IERC721Events.Transfer(CFNFTFF_TREASURY, to, tokenId);
IERC721(CFNFTFF).transferFrom(CFNFTFF_TREASURY, to, tokenId);
vm.stopPrank();
assertEq(IERC721(CFNFTFF).ownerOf(tokenId), to);
Expand All @@ -50,7 +49,7 @@ contract ERC721TokenTest is Test, TestSetup, IERC721Events, IERC20Events {
uint256 tokenId = 1;
vm.startPrank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit Approval(CFNFTFF_TREASURY, spender, tokenId);
emit IERC721Events.Approval(CFNFTFF_TREASURY, spender, tokenId);
IERC721(CFNFTFF).approve(spender, tokenId);
vm.stopPrank();
assertEq(IERC721(CFNFTFF).getApproved(tokenId), spender);
Expand All @@ -60,7 +59,7 @@ contract ERC721TokenTest is Test, TestSetup, IERC721Events, IERC20Events {
address operator = makeAddr("operator");
vm.prank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit ApprovalForAll(CFNFTFF_TREASURY, operator, true);
emit IERC721Events.ApprovalForAll(CFNFTFF_TREASURY, operator, true);
IERC721(CFNFTFF).setApprovalForAll(operator, true);
assertTrue(IERC721(CFNFTFF).isApprovedForAll(CFNFTFF_TREASURY, operator));
}
Expand Down
8 changes: 4 additions & 4 deletions test/HTS.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,7 @@ contract HTSTest is Test, TestSetup {
uint256 serialId = 1;
vm.startPrank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit IERC20Events.Transfer(CFNFTFF_TREASURY, to, serialId);
emit IERC721Events.Transfer(CFNFTFF_TREASURY, to, serialId);
IHederaTokenService(HTS_ADDRESS).transferNFT(CFNFTFF, CFNFTFF_TREASURY, to, int64(int256(serialId)));
vm.stopPrank();
assertEq(IERC721(CFNFTFF).ownerOf(serialId), to);
Expand All @@ -686,7 +686,7 @@ contract HTSTest is Test, TestSetup {
uint256 serialId = 1;
vm.startPrank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit IERC20Events.Transfer(CFNFTFF_TREASURY, to, serialId);
emit IERC721Events.Transfer(CFNFTFF_TREASURY, to, serialId);
IHederaTokenService(HTS_ADDRESS).transferFromNFT(CFNFTFF, CFNFTFF_TREASURY, to, serialId);
vm.stopPrank();
assertEq(IERC721(CFNFTFF).ownerOf(serialId), to);
Expand Down Expand Up @@ -759,7 +759,7 @@ contract HTSTest is Test, TestSetup {
to[0] = makeAddr("recipient");
vm.startPrank(CFNFTFF_TREASURY);
vm.expectEmit(CFNFTFF);
emit IERC20Events.Transfer(CFNFTFF_TREASURY, to[0], serialId[0]);
emit IERC721Events.Transfer(CFNFTFF_TREASURY, to[0], serialId[0]);
IHederaTokenService(HTS_ADDRESS).transferNFT(CFNFTFF, from[0], to[0], int64(int256(serialId[0])));
vm.stopPrank();
assertEq(IERC721(CFNFTFF).ownerOf(serialId[0]), to[0]);
Expand Down Expand Up @@ -800,7 +800,7 @@ contract HTSTest is Test, TestSetup {
assertNotEq(IERC721(token).getApproved(1), newSpender);
vm.prank(CFNFTFF_TREASURY);
vm.expectEmit(token);
emit IERC20Events.Approval(CFNFTFF_TREASURY, newSpender, 1);
emit IERC721Events.Approval(CFNFTFF_TREASURY, newSpender, 1);
int64 responseCodeApprove = HtsSystemContract(HTS_ADDRESS).approveNFT(token, newSpender, 1);
assertEq(responseCodeApprove, HederaResponseCodes.SUCCESS);
assertEq(IERC721(token).getApproved(1), newSpender);
Expand Down