MetaMask / metamask-snaps-beta

Fork of MetaMask that supports plugins! Read the Wiki!
https://github.com/MetaMask/metamask-snaps-beta/wiki
MIT License
142 stars 57 forks source link

custom-accounts signTransaction requires the same instance of ethTx #154

Closed shanejonas closed 2 years ago

shanejonas commented 4 years ago

Further up in the stack it looks ONLY at the reference to ethTx. Which gets lost when passed to a custom-account's eth_signTransaction:

line 447 just passes in ethTx and does not expect any result. just that the reference to ethTx changes

https://github.com/MetaMask/metamask-snaps-beta/blob/220784cafd2e22bed083e385ef247810660966db/app/scripts/controllers/transactions/index.js#L446-L460

Heres some code I changed in my local metamask-snaps-beta directory to get this working with returning rawTransaction (for now, but should probably be the same interface as web3 {raw: "0x0", tx: {}})?

+++ b/app/scripts/controllers/accounts.js
@@ -1,6 +1,8 @@
 const ObservableStore = require('obs-store')
 const EventEmitter = require('safe-event-emitter')
 const sigUtil = require('eth-sig-util')
+const EthTx = require('ethereumjs-tx')
+const ethCommon = require('ethereumjs-common').default
 const normalizeAddress = sigUtil.normalize

 /**
@@ -173,10 +175,15 @@ class AccountsController extends EventEmitter {
       const tx = ethTx.toJSON(true)
       tx.from = fromAddress

-      return handler(this.getOrigin(address), {
+      const rawTransaction = await handler(this.getOrigin(address), {
         method: 'eth_signTransaction',
         params: [tx],
       })
+      const customChainParams = { name: 'custom', chainId: parseInt(ethTx.getChainId(), 16) }
+      const common = ethCommon.forCustomChain(1, customChainParams, 'byzantium')
+      const t = new EthTx(rawTransaction, { common })
+      delete t.from
+      return Object.assign(ethTx, t)
     }
   }

(just using ethCommon here to deal with custom testnets signing, probably not needed)

And heres my snap code calling an API to sign, getting rawTransaction back:

image

rekmarks commented 2 years ago

We don't do custom accounts with the current version of the system and won't fix this issue. Ref: https://docs.metamask.io/guide/snaps.html