TrueFiEng/Waffle

feedback on 4.0

TrejGun opened this issue · 1 comments

Hi there. Thanks a lot for your efforts. I want to give you my feedback on v4. With v3 I had a couple of problems with arrays and BigNumber and I was expected them to be fixed in v4

CASE 1

from https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol#L82

    function balanceOfBatch(address[] memory accounts, uint256[] memory ids)
        public
        view
        virtual
        override
        returns (uint256[] memory)
    {
        require(accounts.length == ids.length, "ERC1155: accounts and ids length mismatch");

        uint256[] memory batchBalances = new uint256[](accounts.length);

        for (uint256 i = 0; i < accounts.length; ++i) {
            batchBalances[i] = balanceOf(accounts[i], ids[i]);
        }

        return batchBalances;
    }

test

    it("should get balance of owner", async function () {
      await this.erc1155Instance.mint(this.owner.address, tokenId, amount, "0x");
      const balances = await this.erc1155Instance.balanceOfBatch(
        [this.owner.address, this.owner.address],
        [tokenId, 0],
      );
      expect(balances).to.deep.equal([BigNumber.from(amount), BigNumber.from(0)]);
    });

FIXED

CASE 2

from my code

  address private _royaltyReceiver;
  uint96 private _royaltyNumerator;

  function getReceiverAndRoyalties() external view returns (address, uint96) {
    return (_royaltyReceiver, _royaltyNumerator);
  }

test

    it.only("Should mint token with royalty 0%", async function () {
      const royaltyNumerator = 5000;
      const tx1 = collectionInstance.setDefaultRoyalty(receiver.address, royaltyNumerator);
      await expect(tx1).to.emit(collectionInstance, "DefaultRoyaltyInfo").withArgs(receiver.address, royaltyNumerator);

      const receiverAndRoyalties = await collectionInstance.getReceiverAndRoyalties();
      expect(receiverAndRoyalties).to.deep.equal([receiver.address, BigNumber.from(royaltyNumerator)]);
    });

FAILS

workaround

      expect(receiverAndRoyalties[0]).to.equal(receiver.address);
      expect(receiverAndRoyalties[1]).to.equal(BigNumber.from(royaltyNumerator));
rzadp commented

Thank you so much for your feedback.
Please take a look at #766
It seems like CASE 2 is working as well? I added a test to make sure.