Skip to content

Commit

Permalink
more tests (Pandora-Labs-Org#16)
Browse files Browse the repository at this point in the history 
* add transferFrom 721 transfer exemption revert tests

* more transferFrom tests

* safe transfer from enforces valid token id
  • Loading branch information
@caldereth
caldereth authored Feb 13, 2024
1 parent ab3178f commit 487294a
Show file tree
Hide file tree
Showing 2 changed files with 238 additions and 2 deletions.
17 changes: 16 additions & 1 deletion contracts/ERC404.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,16 @@ abstract contract ERC404 is IERC404 {
}

/// @notice Function for ERC-721 transfers with contract support.
/// This function only supports moving valid ERC-721 ids, as it does not exist on the ERC-20 spec and will revert otherwise.
function safeTransferFrom(
address from_,
address to_,
uint256 id_
) public virtual {
if (id_ > _minted || id_ == 0) {
revert InvalidId();
}

transferFrom(from_, to_, id_);

if (
Expand All @@ -277,12 +282,17 @@ abstract contract ERC404 is IERC404 {
}

/// @notice Function for ERC-721 transfers with contract support and callback data.
/// This function only supports moving valid ERC-721 ids, as it does not exist on the ERC-20 spec and will revert otherwise.
function safeTransferFrom(
address from_,
address to_,
uint256 id_,
bytes calldata data_
) public virtual {
if (id_ > _minted || id_ == 0) {
revert InvalidId();
}

transferFrom(from_, to_, id_);

if (
Expand Down Expand Up @@ -436,14 +446,17 @@ abstract contract ERC404 is IERC404 {
_owned[from_].pop();
}

// Check if this is a burn.
if (to_ != address(0)) {
// If not a burn, update the owner of the token to the new owner.
// Update owner of the token to the new owner.
_setOwnerOf(id_, to_);
// Push token onto the new owner's stack.
_owned[to_].push(id_);
// Update index for new owner's stack.
_setOwnedIndex(id_, _owned[to_].length - 1);
} else {
// If this is a burn, reset the owner of the token to 0x0 by deleting the token from _ownedData.
delete _ownedData[id_];
}

Expand Down Expand Up @@ -553,6 +566,7 @@ abstract contract ERC404 is IERC404 {
/// @notice Internal function for ERC20 minting
/// @dev This function will allow minting of new ERC20s.
/// If mintCorrespondingERC721s_ is true, and the recipient is not ERC-721 exempt, it will also mint the corresponding ERC721s.
/// Handles ERC-721 exemptions.
function _mintERC20(
address to_,
uint256 value_,
Expand Down Expand Up @@ -623,7 +637,7 @@ abstract contract ERC404 is IERC404 {
// Retrieve the latest token added to the owner's stack (LIFO).
uint256 id = _owned[from_][_owned[from_].length - 1];

// Transfer the token to the contract.
// Transfer to 0x0.
// Does not handle ERC-721 exemptions.
_transferERC721(from_, address(0), id);

Expand All @@ -639,6 +653,7 @@ abstract contract ERC404 is IERC404 {
if (erc20BalanceOf(target_) >= units && !state_) {
revert CannotRemoveFromERC721TransferExempt();
}

erc721TransferExempt[target_] = state_;
}

Expand Down
223 changes: 222 additions & 1 deletion test/ERC404.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,223 @@ describe("ERC404", function () {
).to.be.revertedWithCustomError(f.contract, "InvalidSender")
})

context("Recipient is ERC-721 transfer exempt", function () {
it("Succeeds when transferring as ERC-20", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const from = f.signers[0]
const to = f.signers[3]
const value = (await f.contract.erc721TotalSupply()) + 1n

// Confirm the sender holds sufficient balance.
expect(await f.contract.erc20BalanceOf(from.address)).to.be.gte(value)

await f.contract.setERC721TransferExempt(to.address, true)

// Confirm that the 'to' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(true)

// Confirm that the 'from' address is not ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
false,
)

// 'from' has to grant themselves sufficient allowance to spend their own tokens using transferFrom
await f.contract.connect(from).approve(from.address, value)

// Attempt to send as ERC-20.
return expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, value),
).to.emit(f.contract, "ERC20Transfer")
})

it("Reverts when transferring as ERC-721", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const tokenId = 1n
const from = f.signers[0]
const to = f.signers[3]

await f.contract
.connect(f.signers[0])
.setERC721TransferExempt(to.address, true)

// Confirm that the 'to' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(true)

// Confirm that the 'from' address is not ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
false,
)

// Attempt to send 1 ERC-721.
await expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, tokenId),
).to.be.revertedWithCustomError(
f.contract,
"RecipientIsERC721TransferExempt",
)
})
})

context("Sender is ERC-721 transfer exempt", function () {
it("Succeeds when transferring as ERC-20", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const from = f.signers[0]
const to = f.signers[3]
const value = (await f.contract.erc721TotalSupply()) + 1n

// Confirm the sender holds sufficient balance.
expect(await f.contract.erc20BalanceOf(from.address)).to.be.gte(value)

await f.contract.setERC721TransferExempt(from.address, true)

// Confirm that the 'to' address is not ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(
false,
)

// Confirm that the 'from' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
true,
)

// 'from' has to grant themselves sufficient allowance to spend their own tokens using transferFrom
await f.contract.connect(from).approve(from.address, value)

// Attempt to send as ERC-20.
return expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, value),
).to.emit(f.contract, "ERC20Transfer")
})

it("Reverts when transferring as ERC-721", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const tokenId = 1n
const from = f.signers[0]
const to = f.signers[3]

await f.contract
.connect(f.signers[0])
.setERC721TransferExempt(from.address, true)

// Confirm that the 'to' address is not ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(
false,
)

// Confirm that the 'from' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
true,
)

// Attempt to send 1 ERC-721.
await expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, tokenId),
).to.be.revertedWithCustomError(
f.contract,
"SenderIsERC721TransferExempt",
)
})
})

context(
"Both sender and recipient are ERC-721 transfer exempt",
function () {
it("Succeeds when transferring as ERC-20", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const from = f.signers[0]
const to = f.signers[3]
const value = (await f.contract.erc721TotalSupply()) + 1n

// Confirm the sender holds sufficient balance.
expect(await f.contract.erc20BalanceOf(from.address)).to.be.gte(value)

await f.contract.setERC721TransferExempt(to.address, true)
await f.contract.setERC721TransferExempt(from.address, true)

// Confirm that the 'to' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(
true,
)

// Confirm that the 'from' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
true,
)

// 'from' has to grant themselves sufficient allowance to spend their own tokens using transferFrom
await f.contract.connect(from).approve(from.address, value)

// Attempt to send as ERC-20.
return expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, value),
).to.emit(f.contract, "ERC20Transfer")
})

it("Reverts when transferring as ERC-721", async function () {
const f = await loadFixture(
deployMinimalERC404WithERC20sAndERC721sMinted,
)

const tokenId = 1n
const from = f.signers[0]
const to = f.signers[3]

await f.contract
.connect(f.signers[0])
.setERC721TransferExempt(to.address, true)

await f.contract
.connect(f.signers[0])
.setERC721TransferExempt(from.address, true)

// Confirm that the 'to' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(to.address)).to.equal(
true,
)

// Confirm that the 'from' address is ERC-721 transfer exempt.
expect(await f.contract.erc721TransferExempt(from.address)).to.equal(
true,
)

// Attempt to send 1 ERC-721.
await expect(
f.contract
.connect(from)
.transferFrom(from.address, to.address, tokenId),
).to.be.revertedWithCustomError(
f.contract,
"SenderIsERC721TransferExempt",
)
})
},
)

context("Operator owns the token to be moved", function () {
// This test case proves that the operator cannot use transferFrom to transfer a token they own if they provide the wrong 'from' address.
it("Reverts when attempting to transfer a token that operator owns, but that 'from' does not own", async function () {
Expand Down Expand Up @@ -1722,7 +1939,7 @@ describe("ERC404", function () {
})

describe("E2E tests", function () {
it("Minting out the full supply, making ERC-20 and ERC-721 transfers, banking and retrieving tokens, and setting and removing whitelist addresses", async function () {
it("Minting out the full supply, making ERC-20 and ERC-721 transfers, banking and retrieving tokens", async function () {
const f = await loadFixture(deployMinimalERC404)

// Initial minting. Will mint ERC-20 and ERC-721 tokens.
Expand Down Expand Up @@ -1983,6 +2200,10 @@ describe("ERC404", function () {
"NotFound",
)
})

it("Various weird scenarios where addresses are added and removed from the ERC-721 transfer exempt list", async function () {
// TODO
})
})

describe("#_retrieveOrMintERC721", function () {
Expand Down

0 comments on commit 487294a

Please sign in to comment.