wyvernprotocol / wyvern-v3

Wyvern Protocol v3.1, Ethereum implementation
https://wyvernprotocol.com
MIT License
298 stars 121 forks source link

Error messages and swapOneForOne test #18

Closed alexanderatallah closed 5 years ago

alexanderatallah commented 5 years ago

currently getting Error: Returned error: VM Exception while processing transaction: revert for this test

To make the other NFT test work, we might want to transferFrom the NFTs back in place at the end of this test?

coveralls commented 5 years ago

Pull Request Test Coverage Report for Build 85


Changes Missing Coverage Covered Lines Changed/Added Lines %
contracts/registry/proxy/OwnedUpgradeabilityProxy.sol 3 4 75.0%
<!-- Total: 28 29 96.55% -->
Totals Coverage Status
Change from base Build 83: 0.0%
Covered Lines: 136
Relevant Lines: 142

💛 - Coveralls
alexanderatallah commented 5 years ago

Found from error messages that the proxy wasn't getting created (and then the NFT wasn't being approved) for the second account. Now the call works, but "Static call failed" is the returned solidity error. @cwgoes

alexanderatallah commented 5 years ago

This has something to do with how we're concatenating parameter encoding on the end using encodeParameters, but I haven't figured out what's going on. I also tried this way but still didn't get it to call through (even after commenting out all of the contents of swapOneForOne):

const func = web3.eth.abi.encodeFunctionSignature('swapOneForOne(address[2],uint256[2],address[7],uint8[2],uint256[6],bytes,bytes)')
            const paramTypes = [
              {
                "name": "tokenGiveGet",
                "type": "address[2]"
              },
              {
                "name": "nftGiveGet",
                "type": "uint256[2]"
              }
            ]
            const paramsOne = web3.eth.abi.encodeParameters(
              paramTypes,
              [[erc721.address, erc721.address], [nfts[0], nfts[1]]]
            ).replace('0x', '')
            const paramsTwo = web3.eth.abi.encodeParameters(
              paramTypes,
              [[erc721.address, erc721.address], [nfts[1], nfts[0]]]
            ).replace('0x', '')
cwgoes commented 5 years ago

In fact it was that ABI encoding in Solidity is not positionally-invariant - so we must instead pass all the extra arguments as a bytes parameter to static calls, which luckily isn't too onerous.

Thanks for all the error messages on require!