Skip to content

Commit

Permalink
Fixes in response to ProjectOpenSea/opensea-erc1155 pull request Proj…
Browse files Browse the repository at this point in the history 
…ectOpenSea#17 fedback.
  • Loading branch information
Rob Myers committed May 6, 2020
1 parent c2f4794 commit 02e1660
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 70 deletions.
2 changes: 1 addition & 1 deletion contracts/MyFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ contract MyFactory is IFactory, Ownable, ReentrancyGuard {

/**
* Hack to get things to work automatically on OpenSea.
* Use safTeransferFrom so the frontend doesn't have to worry about different method names.
* Use safeTransferFrom so the frontend doesn't have to worry about different method names.
*/
function safeTransferFrom(
address /* _from */,
Expand Down
17 changes: 17 additions & 0 deletions lib/testValuesCommon.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/* Useful aliases */

const toBN = web3.utils.toBN;


const URI_BASE = 'https://opensea-creatures-api.herokuapp.com/api/';
const ADDRESS_ZERO = '0x0000000000000000000000000000000000000000';
const MAX_UINT256 = '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF';
const MAX_UINT256_BN = toBN(MAX_UINT256);


module.exports = {
URI_BASE,
ADDRESS_ZERO,
MAX_UINT256,
MAX_UINT256_BN
};
95 changes: 57 additions & 38 deletions test/ERC1155Tradable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

const truffleAssert = require('truffle-assertions');

const vals = require('../lib/testValuesCommon.js');


/* Contracts in this test */

Expand All @@ -19,13 +21,10 @@ const toBN = web3.utils.toBN;
contract("ERC1155Tradable - ERC 1155", (accounts) => {
const NAME = 'ERC-1155 Test Contract';
const SYMBOL = 'ERC1155Test';
const URI_ROOT = 'https://a.b.c/def/hji/';
const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000';

const INITIAL_TOKEN_ID = 1;
const NON_EXISTENT_TOKEN_ID = 99999999;
const INITIAL_TOKEN_SUPPLY = 2;
const MINT_AMOUNT = 2;
const MINT_AMOUNT = toBN(100);

const OVERFLOW_NUMBER = toBN(2, 10).pow(toBN(256, 10)).sub(toBN(1, 10));

Expand All @@ -39,7 +38,11 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
let proxy;

// Keep track of token ids as we progress through the tests, rather than
// hardcoding numbers that we will haev to change if we add/move tests.
// hardcoding numbers that we will have to change if we add/move tests.
// For example if test A assumes that it will create token ID 1 and test B
// assumes that it will create token 2, changing test A later so that it
// creates another token will break this as test B will now create token ID 3.
// Doing this avoids this scenario.
let tokenId = 0;

// Because we need to deploy and use a mock ProxyRegistry, we deploy our own
Expand Down Expand Up @@ -70,7 +73,7 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
'TransferSingle',
{
_operator: owner,
_from: ZERO_ADDRESS,
_from: vals.ADDRESS_ZERO,
_to: owner,
_id: toBN(tokenId),
_amount: toBN(0)
Expand All @@ -84,18 +87,24 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
async () => {
tokenId += 1;
truffleAssert.eventEmitted(
await instance.create(owner, 100, "", "0x0", { from: owner }),
await instance.create(
owner,
MINT_AMOUNT,
"",
"0x0",
{ from: owner }
),
'TransferSingle',
{
_operator: owner,
_from: ZERO_ADDRESS,
_from: vals.ADDRESS_ZERO,
_to: owner,
_id: toBN(tokenId),
_amount: toBN(100)
_amount: MINT_AMOUNT
}
);
const supply = await instance.tokenSupply(tokenId);
assert.ok(supply.eq(toBN(100)));
assert.ok(supply.eq(MINT_AMOUNT));
});

// We check some of this in the other create() tests but this makes it
Expand Down Expand Up @@ -147,10 +156,16 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
async () => {
tokenId += 1;
truffleAssert.eventEmitted(
await instance.create(owner, 0, URI_ROOT, "0x0", { from: owner }),
await instance.create(
owner,
0,
vals.URI_BASE,
"0x0",
{ from: owner }
),
'URI',
{
_uri: URI_ROOT,
_uri: vals.URI_BASE,
_id: toBN(tokenId)
}
);
Expand All @@ -170,25 +185,27 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
it('should return correct value for token supply',
async () => {
tokenId += 1;
await instance.create(owner, 100, "", "0x0", { from: owner });
await instance.create(owner, MINT_AMOUNT, "", "0x0", { from: owner });
const balance = await instance.balanceOf(owner, tokenId);
assert.ok(balance.eq(toBN(100)));
assert.ok(balance.eq(MINT_AMOUNT));
// Use the created getter for the map
const supplyGetterValue = await instance.tokenSupply(tokenId);
assert.ok(supplyGetterValue.eq(toBN(100)));
assert.ok(supplyGetterValue.eq(MINT_AMOUNT));
// Use the hand-crafted accessor
const supplyAccessorValue = await instance.totalSupply(tokenId);
assert.ok(supplyAccessorValue.eq(toBN(100)));
assert.ok(supplyAccessorValue.eq(MINT_AMOUNT));
// Make explicitly sure everything mateches
assert.ok(supplyGetterValue.eq(balance));
assert.ok(supplyAccessorValue.eq(balance));
});

it('should return zero for non-existent token',
async () => {
await truffleAssert.passes(
instance.balanceOf(owner, NON_EXISTENT_TOKEN_ID)
const balanceValue = await instance.balanceOf(
owner,
NON_EXISTENT_TOKEN_ID
);
assert.ok(balanceValue.eq(toBN(0)));
const supplyAccessorValue = await instance.totalSupply(
NON_EXISTENT_TOKEN_ID
);
Expand All @@ -214,7 +231,7 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
it('should not allow the token creator to set creator to 0x0',
() => truffleAssert.fails(
instance.setCreator(
ZERO_ADDRESS,
vals.ADDRESS_ZERO,
[INITIAL_TOKEN_ID],
{ from: creator }
),
Expand Down Expand Up @@ -249,27 +266,27 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
{ from: creator }
);
let supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.equal(supply, MINT_AMOUNT);
assert.isOk(supply.eq(MINT_AMOUNT));
});

it('should update token totalSupply when minting', async () => {
let supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.equal(supply, MINT_AMOUNT);
await instance.mint(
userA,
INITIAL_TOKEN_ID,
MINT_AMOUNT,
"0x0",
{ from: creator }
);
supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.equal(supply, MINT_AMOUNT * 2);
let supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.isOk(supply.eq(MINT_AMOUNT));
await instance.mint(
userA,
INITIAL_TOKEN_ID,
MINT_AMOUNT,
"0x0",
{ from: creator }
);
supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.isOk(supply.eq(MINT_AMOUNT.mul(toBN(2))));
});

it('should not overflow token balances',
async () => {
const supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.equal(supply, INITIAL_TOKEN_SUPPLY + MINT_AMOUNT);
assert.isOk(supply.eq(MINT_AMOUNT.add(MINT_AMOUNT)));
await truffleAssert.fails(
instance.mint(
userB,
Expand All @@ -295,7 +312,9 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
{ from: creator }
);
const supply = await instance.totalSupply(INITIAL_TOKEN_ID);
assert.equal(supply, INITIAL_TOKEN_SUPPLY + (MINT_AMOUNT * 2));
assert.isOk(
supply.eq(MINT_AMOUNT.mul(toBN(3)))
);
});

it('should not overflow token balances',
Expand Down Expand Up @@ -329,22 +348,22 @@ contract("ERC1155Tradable - ERC 1155", (accounts) => {
describe ('#setBaseMetadataURI()', () => {
it('should allow the owner to set the base metadata url', async () =>
truffleAssert.passes(
instance.setBaseMetadataURI(URI_ROOT, { from: owner })
instance.setBaseMetadataURI(vals.URI_BASE, { from: owner })
));

it('should not allow non-owner to set the base metadata url', async () =>
truffleAssert.fails(
instance.setBaseMetadataURI(URI_ROOT, { from: userA }),
instance.setBaseMetadataURI(vals.URI_BASE, { from: userA }),
truffleAssert.ErrorType.revert,
'Ownable: caller is not the owner'
));
});

describe ('#uri()', () => {
it('should return the correct uri for a token', async () => {
const tokenId = 1;
const uri = await instance.uri(tokenId);
assert.equal(uri, `${URI_ROOT}${tokenId}`);
const uriTokenId = 1;
const uri = await instance.uri(uriTokenId);
assert.equal(uri, `${vals.URI_BASE}${uriTokenId}`);
});

it('should not return the uri for a non-existent token', async () =>
Expand Down
29 changes: 13 additions & 16 deletions test/MyFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const truffleAssert = require('truffle-assertions');

const vals = require('../lib/testValuesCommon.js');

/* Contracts in this test */

Expand Down Expand Up @@ -35,10 +36,6 @@ const toBN = web3.utils.toBN;
*/

contract("MyFactory", (accounts) => {
const URI_BASE = 'https://opensea-creatures-api.herokuapp.com/api/';
const ADDRESS_ZERO = '0x0000000000000000000000000000000000000000';
const MAX_UINT256 = '0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF';
const MAX_UINT256_BN = toBN(MAX_UINT256);
// As set in (or inferred from) the contract
const BASIC = 0;
const PREMIUM =1;
Expand Down Expand Up @@ -132,7 +129,7 @@ contract("MyFactory", (accounts) => {
assert.isOk(balanceUserA.eq(quantity));
// Check that balance is correct
const balanceOf = await myFactory.balanceOf(owner, PREMIUM);
assert.isOk(balanceOf.eq(MAX_UINT256_BN.sub(quantity)));
assert.isOk(balanceOf.eq(vals.MAX_UINT256_BN.sub(quantity)));
// Check that total supply is correct
const totalSupply = await myCollectible.totalSupply(PREMIUM);
assert.isOk(totalSupply.eq(quantity));
Expand All @@ -154,7 +151,7 @@ contract("MyFactory", (accounts) => {
assert.isOk(balanceUserB.eq(quantity));
// Check that balance is correct
const balanceOf = await myFactory.balanceOf(owner, GOLD);
assert.isOk(balanceOf.eq(MAX_UINT256_BN.sub(total)));
assert.isOk(balanceOf.eq(vals.MAX_UINT256_BN.sub(total)));
// Check that total supply is correct
const totalSupply1 = await myCollectible.totalSupply(2);
assert.isOk(totalSupply1.eq(total));
Expand All @@ -176,7 +173,7 @@ contract("MyFactory", (accounts) => {
assert.isOk(balanceUserA.eq(total));
// Check that balance is correct
const balanceOf = await myFactory.balanceOf(owner, PREMIUM);
assert.isOk(balanceOf.eq(MAX_UINT256_BN.sub(total)));
assert.isOk(balanceOf.eq(vals.MAX_UINT256_BN.sub(total)));
// Check that total supply is correct
const totalSupply = await myCollectible.totalSupply(PREMIUM);
assert.isOk(totalSupply.eq(total));
Expand Down Expand Up @@ -211,29 +208,29 @@ contract("MyFactory", (accounts) => {

describe('#uri()', () => {
it('should return the correct uri for an option', async () =>
assert.equal(await myFactory.uri(BASIC), `${URI_BASE}factory/0`)
assert.equal(await myFactory.uri(BASIC), `${vals.URI_BASE}factory/0`)
);

it('should format any number as an option uri', async () =>
assert.equal(
await myFactory.uri(MAX_UINT256),
`${URI_BASE}factory/${toBN(MAX_UINT256).toString()}`
await myFactory.uri(vals.MAX_UINT256),
`${vals.URI_BASE}factory/${toBN(vals.MAX_UINT256).toString()}`
));
});

describe('#balanceOf()', () => {
it('should return max supply for un-minted token', async () => {
const balanceOwner = await myFactory.balanceOf(owner, BASIC);
assert.isOk(balanceOwner.eq(MAX_UINT256_BN));
assert.isOk(balanceOwner.eq(vals.MAX_UINT256_BN));
const balanceProxy = await myFactory.balanceOf(
proxyForOwner,
NO_SUCH_OPTION
);
assert.isOk(balanceProxy.eq(MAX_UINT256_BN));
assert.isOk(balanceProxy.eq(vals.MAX_UINT256_BN));
});

it('should return balance of minted token', async () => {
const balance = MAX_UINT256_BN.sub(toBN(1100));
const balance = vals.MAX_UINT256_BN.sub(toBN(1100));
const balanceOwner = await myFactory.balanceOf(owner, PREMIUM);
assert.isOk(balanceOwner.eq(balance));
const balanceProxy = await myFactory.balanceOf(proxyForOwner, PREMIUM);
Expand All @@ -257,7 +254,7 @@ contract("MyFactory", (accounts) => {
const amount = toBN(100);
const userBBalance = await myCollectible.balanceOf(userB, PREMIUM);
await myFactory.safeTransferFrom(
ADDRESS_ZERO,
vals.ADDRESS_ZERO,
userB,
PREMIUM,
amount,
Expand All @@ -271,7 +268,7 @@ contract("MyFactory", (accounts) => {
const amount = toBN(100);
const userBBalance = await myCollectible.balanceOf(userB, PREMIUM);
await myFactory.safeTransferFrom(
ADDRESS_ZERO,
vals.ADDRESS_ZERO,
userB,
PREMIUM,
100,
Expand All @@ -286,7 +283,7 @@ contract("MyFactory", (accounts) => {
const amount = toBN(100);
await truffleAssert.fails(
myFactory.safeTransferFrom(
ADDRESS_ZERO,
vals.ADDRESS_ZERO,
userB,
PREMIUM,
amount,
Expand Down
Loading

0 comments on commit 02e1660

Please sign in to comment.