trufflesuite / truffle

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
14.02k stars 2.32k forks source link

Support signTypedData_v4 in HDWalletProvider #5220

Closed sunwrobert closed 2 years ago

sunwrobert commented 2 years ago

Issue

OpenSea recently launched the Seaport protocol, which uses EIP-712 signatures and has arrays in the payload. It doesn't seem that HDWalletProvider supports signTypedData_v4, so people trying to sign the order either get the following errors:

  1. 'The method eth_signTypedData_v4 does not exist/is not available'
  2. Error: Arrays are unimplemented in encodeData; use V4 extension

Here are some relevant issues: https://github.com/ProjectOpenSea/opensea-js/issues/588 https://github.com/ProjectOpenSea/seaport-js/issues/52

Steps to Reproduce

Please provide the shortest amount of steps to reproduce your issue.

Expected Behavior

What you expected to happen.

Actual Results

What actually happened. Please give examples and support it with screenshots, copied output or error messages.

Environment

gnidan commented 2 years ago

Can you just use Truffle Dashboard instead?

Truffle Dashboard should support eth_signTypedData_v4 just fine (I tested it a bunch), and we've been considering deprecating hdwallet-provider because of the security risk when sharing private keys with Node.js runtime (there's no known issue, but the paranoia is real 😵‍💫)

sunwrobert commented 2 years ago

Hmm, I can suggest that as a solution for those using HDWalletProvider. I'm assuming most of them are running them as part of some script though, so not exactly sure how it'll work with Truffle dashboard.

Is there a programattic alternative to HDWalletProvider? I'm worried that Truffle dashboard wouldn't solve the needs of the users primarily using HDWalletProvider

gnidan commented 2 years ago

Hmm, I can suggest that as a solution for those using HDWalletProvider. I'm assuming most of them are running them as part of some script though, so not exactly sure how it'll work with Truffle dashboard.

Yeah, it might be a substantial change for existing workflows. Just offering it as a workaround. We'll keep this issue open as a bug until we can get it triaged/fixed.

nextmove commented 2 years ago

I post thousands of orders per day. Signing needs to be automatic. Can Truffle Dashboard do that or does it require a manual click?

gnidan commented 2 years ago

I post thousands of orders per day. Signing needs to be automatic. Can Truffle Dashboard do that or does it require a manual click?

@nextmove yeah that's a very compelling argument against deprecating hdwallet-provider!

The answer to your question is the same as "will MetaMask ever allow applications to request signatures without prompting the user?", and I'm sure the answer to that question is "uhhh that sounds like a bad idea"

Ghijo1 commented 2 years ago

So OpenSea changes protocol and adopts one in which orders cannot be entered programmatically. But it raises the problem to Truffle. I do not know whether to laugh or cry.

can you tell me which alternative to use opensea-js to place orders via the API?

sunwrobert commented 2 years ago

@Ghijo1 the alternative you will need is to find a wallet provider that can support signTypedData_v4. If one does not exist, then one needs to be built.

@gnidan are you familiar with any such provider?

gnidan commented 2 years ago

@gnidan are you familiar with any such provider?

I'm not. I don't know much about alternatives to hdwallet-provider. Seems like we should just (a) never deprecate the package, and (b) add this functionality.

Racherin commented 2 years ago

@gnidan are you familiar with any such provider?

I'm not. I don't know much about alternatives to hdwallet-provider. Seems like we should just (a) never deprecate the package, and (b) add this functionality.

first b, then a 😄

0R61 commented 2 years ago

This needs to be added

eggplantzzz commented 2 years ago

Will you please provide a usage example illustrating how you would expect to use this feature with this package? I'll go ahead and implement this if you can let me know. Also, would someone be willing to test this for me afterwards?

Racherin commented 2 years ago

Will you please provide a usage example illustrating how you would expect to use this feature with this package? I'll go ahead and implement this if you can let me know. Also, would someone be willing to test this for me afterwards?

Hey, I already added code block that throws the v4 error here.

https://github.com/ProjectOpenSea/seaport-js/issues/52

you can reach me to test anything about this issue 😊

0R61 commented 2 years ago

Will you please provide a usage example illustrating how you would expect to use this feature with this package? I'll go ahead and implement this if you can let me know. Also, would someone be willing to test this for me afterwards?

Yes I can perform tests

0R61 commented 2 years ago

Will you please provide a usage example illustrating how you would expect to use this feature with this package? I'll go ahead and implement this if you can let me know. Also, would someone be willing to test this for me afterwards?

We need this to be able to submit offers using the new opensea api.

MetalGear-cmd commented 2 years ago

Any update on this? Is this feature going to be added?

eggplantzzz commented 2 years ago

I just published version 2.0.10-alpha.0 of @truffle/hdwallet-provider with my updates to the signing method. I don't really have an easy way right now to test this method. If you all have a chance to test it and give me some feedback I would appreciate it!

Racherin commented 2 years ago

@eggplantzzz still same error on me

eggplantzzz commented 2 years ago

Are you sure it is the same error? If it is exactly the same then I assume you didn't change the package version since the stacktrace should be different as eth-sig-util was replaced with @metamask/eth-sig-util. Can you give me the full error? Make sure you have @truffle/hdwallet-provider@2.0.10-alpha.0 installed.

0R61 commented 2 years ago

Testing now

0R61 commented 2 years ago

Are you sure it is the same error? If it is exactly the same then I assume you didn't change the package version since the stacktrace should be different as eth-sig-util was replaced with @metamask/eth-sig-util. Can you give me the full error? Make sure you have @truffle/hdwallet-provider@2.0.10-alpha.0 installed.

node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:164
    if (results.has(primaryType) || types[primaryType] === undefined) {
                                         ^

TypeError: Cannot read properties of undefined (reading 'EIP712Domain')
    at findTypeDependencies (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:164:42)
    at encodeType (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:140:26)
    at hashType (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:194:37)
    at encodeData (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:120:28)
    at hashStruct (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:184:37)
    at Object.eip712Hash (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:230:16)
    at Object.signTypedData (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@metamask/eth-sig-util/dist/sign-typed-data.js:329:34)
    at HookedWalletSubprovider.signTypedMessage (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/@truffle/hdwallet-provider/dist/index.js:217:50)
    at /home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/web3-provider-engine/subproviders/hooked-wallet.js:390:18
    at nextTask (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/async/waterfall.js:16:14)
    at next (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/async/waterfall.js:23:9)
    at /home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/async/internal/onlyOnce.js:12:16
    at HookedWalletSubprovider.checkApproval (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/web3-provider-engine/subproviders/hooked-wallet.js:403:3)
    at /home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/web3-provider-engine/subproviders/hooked-wallet.js:389:30
    at nextTask (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/async/waterfall.js:16:14)
    at next (/home/notjeff/Desktop/OPENSEABOTSERVER/seabot/node_modules/async/waterfall.js:23:9)
eggplantzzz commented 2 years ago

Thanks @0R61, I'll see if I can figure anything else out about this. Are others getting the same results?

0R61 commented 2 years ago

Thanks @0R61, I'll see if I can figure anything else out about this. Are others getting the same results?

Yeah, they are. OpenSea disabled their previous way of handling offers which leaves developers unable to send in programmatic offers due to this issue with hdwallet-provider. Would you mind adding me on discord so that we can have a quicker turn around, I can help with testing and fixing as well.

My discord: X_#1368

MetalGear-cmd commented 2 years ago

Same error as before, plus the eth_signTypedData_v4 was coming when using 0xsubprovider TypeError: Cannot read properties of undefined (reading 'EIP712Domain') at findTypeDependencies (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:288:40) at encodeType (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:256:24) at hashType (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:331:17) at encodeData (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:224:37) at hashStruct (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:317:17) at Object.eip712Hash (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:377:5) at Object.signTypedData (C:\testing\node_modules\@metamask\eth-sig-util\src\sign-typed-data.ts:512:24) at HookedWalletSubprovider.signTypedMessage (C:\testing\node_modules\@truffle\hdwallet-provider\src\index.ts:230:29) at C:\testing\node_modules\@truffle\hdwallet-provider\node_modules\web3-provider-engine\subproviders\hooked-wallet.js:390:18 at nextTask (C:\testing\node_modules\async\waterfall.js:16:14) at next (C:\testing\node_modules\async\waterfall.js:23:9) at C:\testing\node_modules\async\internal\onlyOnce.js:12:16 at HookedWalletSubprovider.checkApproval (C:\testing\node_modules\@truffle\hdwallet-provider\node_modules\web3-provider-engine\subproviders\hooked-wallet.js:403:3) at C:\testing\node_modules\@truffle\hdwallet-provider\node_modules\web3-provider-engine\subproviders\hooked-wallet.js:389:30 at nextTask (C:\testing\node_modules\async\waterfall.js:16:14) at next (C:\testing\node_modules\async\waterfall.js:23:9)

0R61 commented 2 years ago

I resolved the issue, will do a commit soon.

MetalGear-cmd commented 2 years ago

Thanks alot Will be waiting for it.

gnidan commented 2 years ago

I had this old gnidan/guestbook-712 repo just lying around, and I just showed it to @eggplantzzz, and we think it should suffice for internal testing.

If anyone's particularly sharp with EIP-712, could you please take a look at the JS util for requesting eth_signTypedData_v4 and the corresponding Solidity validation and let us know if that looks correct? Not sure if the standard has changed at all, or if I wrote it correctly in the first place.

Thanks!

0R61 commented 2 years ago

https://github.com/0R61/pwned-hdwalletprovider

0R61 commented 2 years ago

This supports the signTypedData_v4

gnidan commented 2 years ago

https://github.com/0R61/pwned-hdwalletprovider

This supports the signTypedData_v4

cool, thanks @0R61! seems like we'll be able to converge quickly on a proper solution in @truffle/hdwallet-provider itself.

regarding release timing, it's looking most likely that this will be done in time for next week's release, since this week's release is scheduled for today at 4pm ET (GMT-4)... which is in about 2 hours. seems low odds to have a PR tested, approved, and merged before then :/

0R61 commented 2 years ago

Yeah the actual "fix" relies in @metamask eth-sig-util in the sign-typed-data.js file:

function sanitizeData(unparsedData) {
    const data=JSON.parse(unparsedData)
    const sanitizedData = {};
    for (const key in exports.TYPED_MESSAGE_SCHEMA.properties) {
        if (data[key]) {
            sanitizedData[key] = data[key];
        }
    }
    if ('types' in sanitizedData) {
        sanitizedData.types = Object.assign({ EIP712Domain: [] }, sanitizedData.types);
    }
    return sanitizedData;
}

The sanitizeData function was not parsing JSON string resulting in everything being sanitized by the function.

0R61 commented 2 years ago

@gnidan

MetalGear-cmd commented 2 years ago

@0R61 I am seeing this error opensea-js v 4.0.3 node version: 16.13.2 { code: -32601, message: 'The method eth_signTypedData_v4 does not exist/is not available' }

gnidan commented 2 years ago

Yeah the actual "fix" relies in @MetaMask eth-sig-util in the sign-typed-data.js file:

function sanitizeData(unparsedData) {
    const data=JSON.parse(unparsedData)
    const sanitizedData = {};
    for (const key in exports.TYPED_MESSAGE_SCHEMA.properties) {
        if (data[key]) {
            sanitizedData[key] = data[key];
        }
    }
    if ('types' in sanitizedData) {
        sanitizedData.types = Object.assign({ EIP712Domain: [] }, sanitizedData.types);
    }
    return sanitizedData;
}

The sanitizeData function was not parsing JSON string resulting in everything being sanitized by the function.

@0R61 does the release version of @metamask/eth-sig-util behave correctly, or does that need a fix? sorry if I missed that in the conversation above. I took a quick look, and it seems like the current implementation is what you wrote.

0R61 commented 2 years ago

@0R61 I am seeing this error opensea-js v 4.0.3 node version: 16.13.2 { code: -32601, message: 'The method eth_signTypedData_v4 does not exist/is not available' }

Please send full error message.

0R61 commented 2 years ago

Yeah the actual "fix" relies in @MetaMask eth-sig-util in the sign-typed-data.js file:

function sanitizeData(unparsedData) {
    const data=JSON.parse(unparsedData)
    const sanitizedData = {};
    for (const key in exports.TYPED_MESSAGE_SCHEMA.properties) {
        if (data[key]) {
            sanitizedData[key] = data[key];
        }
    }
    if ('types' in sanitizedData) {
        sanitizedData.types = Object.assign({ EIP712Domain: [] }, sanitizedData.types);
    }
    return sanitizedData;
}

The sanitizeData function was not parsing JSON string resulting in everything being sanitized by the function.

@0R61 does the release version of @metamask/eth-sig-util behave correctly, or does that need a fix? sorry if I missed that in the conversation above. I took a quick look, and it seems like the current implementation is what you wrote.

I will perform tests now.

0R61 commented 2 years ago

No the current release version of eth-sig-util version 4.0.1 is giving the same error. My implementation is different, I believe.

MetalGear-cmd commented 2 years ago

@0R61 I am seeing this error opensea-js v 4.0.3 node version: 16.13.2 { code: -32601, message: 'The method eth_signTypedData_v4 does not exist/is not available' }

Please send full error message.

This is the full message I see the exact same message when trying to use the 0xsubprovider. My node provider in Infura, I see you have used moralis's speedy node

0R61 commented 2 years ago

@0R61 I am seeing this error opensea-js v 4.0.3 node version: 16.13.2 { code: -32601, message: 'The method eth_signTypedData_v4 does not exist/is not available' }

Please send full error message.

This is the full message I see the exact same message when trying to use the 0xsubprovider. My node provider in Infura, I see you have used moralis's speedy node

Try using moralis. Did you replace @metamask folder and @truffle folder with the ones in my repo?

MetalGear-cmd commented 2 years ago

yes I replaced it and also tried the moralis speedy node, it just exits without doing anything, same thing happens with Alchemy.

gnidan commented 2 years ago

No the current release version of eth-sig-util version 4.0.1 is giving the same error. My implementation is different, I believe.

Hm... I don't see any functional difference between what you wrote and what's on main, which appears to be in 4.0.1 as well. I hope @metamask/eth-sig-util is fine, otherwise we're dealing with getting a fix released there first...

Maybe @eggplantzzz will have more findings once he tests this a bit further.

0R61 commented 2 years ago

yes I replaced it and also tried the moralis speedy node, it just exits without doing anything, same thing happens with Alchemy.

Post your code

MetalGear-cmd commented 2 years ago
const NODE_API_KEY = "Infura API";
const OWNER_ADDRESS = "My Wallet Address"; 
const API_KEY = "OpenSea API Key";
const HDWalletProvider = require("@pwned/hdwallet-provider");
const NETWORK ="mainnet";
const { OpenSeaSDK, Network } = require("opensea-js");
const { WyvernSchemaName } = require("opensea-js/lib/types");
const privateKey ="7347311ddbce...........";
const providerOrUrl = "https://" + NETWORK + ".infura.io/v3/" + NODE_API_KEY;

const provider = new HDWalletProvider({privateKeys:[privateKey], provider: providerOrUrl});
const openseaSDK = new OpenSeaSDK(provider, {
  networkName: Network.Main,
  apiKey: API_KEY,
});
async function main() {
  try {
    let nftInfoObjectArray = [
      {
        tokenAddress: "0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85",
        token_id: "62583401953343851428639401407982261461988733337497765194058172228456582339030",
      },
      {
        tokenAddress: "0x57f1887a8bf19b14fc0df6fd9b2acc9af147ea85",
        token_id: "71849748206808787525715296396661270752601310545010038787801650881895754327583",
      },     
    ];
      for(let oof=0;oof<3-2;oof++){
        const offer = await openseaSDK.createBuyOrder({
          asset: {
            tokenId: nftInfoObjectArray[oof].token_id,
            tokenAddress: nftInfoObjectArray[oof].tokenAddress,
            schemaName: WyvernSchemaName.ERC721,
          },
          expirationTime: Math.round(Date.now() / 1000 + 60 * 60 *1),
          accountAddress: OWNER_ADDRESS,
          startAmount: 0.00000003,
        });
        console.log(offer);
      }
  } catch (e) {
    console.log(e);
  }
}
main();
0R61 commented 2 years ago

@gnidan Yeah unfortunately, I think that is where the issue resides. I opened an issue with them.

eggplantzzz commented 2 years ago

Just for reference, https://github.com/MetaMask/eth-sig-util/issues/257

MetalGear-cmd commented 2 years ago

@0R61 Getting this error now after re-installing all the modules againg and replacing yours with the already existant:

Error: API Error 400: You cannot access body after reading from request's data stream at OpenSeaAPI. (C:\testing\node_modules\opensea-js\lib\api.js:592:31) at step (C:\testing\node_modules\opensea-js\lib\api.js:63:23) at Object.next (C:\testing\node_modules\opensea-js\lib\api.js:44:53) at fulfilled (C:\testing\node_modules\opensea-js\lib\api.js:35:58) at processTicksAndRejections (node:internal/process/task_queues:96:5)

When will the nightmare end.

0R61 commented 2 years ago

@0R61 Getting this error now after re-installing all the modules againg and replacing yours with the already existant:

Error: API Error 400: You cannot access body after reading from request's data stream

at OpenSeaAPI.<anonymous> (C:\testing\node_modules\opensea-js\lib\api.js:592:31)

at step (C:\testing\node_modules\opensea-js\lib\api.js:63:23)

at Object.next (C:\testing\node_modules\opensea-js\lib\api.js:44:53)

at fulfilled (C:\testing\node_modules\opensea-js\lib\api.js:35:58)

at processTicksAndRejections (node:internal/process/task_queues:96:5)

When will the nightmare end.

This is an issue within opensea-js.

I will post a modified version of their api as a repo tomorrow

eggplantzzz commented 2 years ago

So the problem may be on the Truffle side after all. I published a new version of the package, @truffle/hdwallet-provider@2.0.10-alpha.2. Can you test it out for me? It seems to test successfully with a toy project I have here.

The issue might be that we need to convert the data (that has been serialized/stringified) back into an object before feeding it to the signTypedData utility that MetaMask provides us. Let me know what results you get for this new version.

odanado commented 2 years ago

@eggplantzzz I have confirmed with @truffle/hdwallet-provider@2.0.10-alpha.2 that the problem has been fixed! Thanks!

Here is the source code I tried. https://github.com/ProjectOpenSea/opensea-js/issues/550#issuecomment-1165747035

nextmove commented 2 years ago

I was able to post an order to Seaport after installing @truffle/hdwallet-provider@2.0.10-alpha.2. Thank you @eggplantzzz !

BeniSchlattmann commented 2 years ago

Hi All I'm still running into the same issue with the following test script.

Running: └── opensea-js@4.0.3 └── @truffle/hdwallet-provider@2.0.10-alpha.0 node version: v14.17.6

Error: (node:9318) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'EIP712Domain' of undefined

Test Code:

import * as Web3 from 'web3'
import { OpenSeaSDK, Network, WyvernProtocol } from 'opensea-js'
import { WyvernSchemaName } from "opensea-js/lib/types"
const HDWalletProvider = require("@truffle/hdwallet-provider");

async function test(){
    const PRIVATE_KEY = "privatekey";
    const accountAddress="publicaddr";
    const providerUrl="https://mainnet.infura.io/v3/apikey"
    const OSAPIKey = 'osapikey';

    let provider = new HDWalletProvider({
        privateKeys: [
            PRIVATE_KEY
        ],
        providerOrUrl: providerUrl
    });
    const seaport = new OpenSeaSDK(provider, {
        networkName: Network.Main,
        apiKey: OSAPIKey
    });

    const offer = await seaport.createBuyOrder({
    asset: {
        tokenId: "987",
        tokenAddress: "0x80a4b80c653112b789517eb28ac111519b608b19",
        schemaName: WyvernSchemaName.ERC721,
    },
    accountAddress: accountAddress,
    startAmount: 0.0001,
    expirationTime: Math.round(Date.now() / 1000 + 3600000 / 1000)
    })
}
test();

Any advice what I'm doing wrong here?