MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.95k stars 4.89k forks source link

Walletconnect signRawTransaction is not implemented #7711

Open kikoncuo opened 4 years ago

kikoncuo commented 4 years ago

What problem are you trying to solve? Currently the functionality for metamask to request the signature of a raw transaction and return it using walletconnect is not enabled.

Describe the solution you'd like implement the eth_signTransaction endpoint

kikoncuo commented 4 years ago

Maybe it´s important to note that the rest of the walletconnect implementation is there. Why did they choose to leave that out?

luziusmeisser commented 3 years ago

I think this is related to the following issue: https://github.com/MetaMask/metamask-extension/issues/3475

In short, the MetaMask devs believe that only MetaMask can manage the nonce in a sensible matter and every other developer interacting with MetaMask is too stupid to get the nonce correctly, therefore, they choose to not implement eth_signTransaction despite it being part of the official web3 API.

I would guess that the same logic applies here and I would not expect MetaMask to ever implement this feature. The best solution is to tell your users to use a better wallet.

pi0neerpat commented 3 years ago

Related https://github.com/ethers-io/ethers.js/issues/957

danfinlay commented 3 years ago

In short, the MetaMask devs believe that only MetaMask can manage the nonce in a sensible matter and every other developer interacting with MetaMask is too stupid to get the nonce correctly, therefore, they choose to not implement eth_signTransaction despite it being part of the official web3 API.

The problem is NOT that we believe "every developer is too stupid". The problem is that with this proposal, it only takes ONE developer who mis-manages a nonce to leave a user's wallet in a stuck state, (and we've seen this before, back when we allowed dapp-specified nonces), and we are then left to support that user, and our support is already backlogged heavily.

Rather than write accusatory attacks, I think it would be a lot more productive to talk about the use cases that are motivating this request, and see if there are ways those needs can be met safely, in ways that users aren't exposed to clogged wallet risks.

So please:

Tell us WHY you'd like to see this feature!

luziusmeisser commented 3 years ago

Sorry for the tone of the previous message and nice to hear from you.

First, let me point out that as long as MetaMask offers a possibility to export private keys, the problem of clogged wallets cannot be ruled out. I myself am using my everyday private key on multiple devices, wallets and browsers. The only way to prevent clogged wallets is to closely watch the blockchain for pending and completed transactions.

Second, here is a description of our use-case: we have implemented a multi-signature scheme that allows users to sign transactions as if they were normal, single-signature transactions. This allows wallets to correctly show to the user what the transaction is about. Here is an example transaction:

https://etherscan.io/tx/0x092feb3c8c8a71a3ce242e2295d8ec2c9988586c9a54c75f05d63297d1688995

Here, two users signed a normal ERC-20 transfer transaction to send 80'000 XCHF from the multisignature address. Both users could connect to our frontend through Walletconnect and their wallets could tell them what they are signing, namely an ERC-20 transfer. With other multisignature approaches, the users have to sign cryptic transactions that do not immediately reveal what they are about. After the users have signed their transactions, we collect them and send them as one big multisignature transaction to the multisig contract once the necessary number of signature is reached (in this case two). The signatures of the users are then verified by the multisig contract and the transaction actually executed. As a nice side-effect, this also allows us to pay for the gas fees of our clients, so they do not have to bother with acquiring Ether.

You can find more documentation including a link to the source code here: https://aktionariat.com/documentation/smart-contracts/multisig.html

I recognize that this is a special case and we are maybe the only ones in the world doing it that way. Nonetheless, it would be nice to be able to tell our client that our solution works with any Walletconnect compatible wallet, including Metamask.

pi0neerpat commented 3 years ago

I'd like to be able to schedule transactions in the future. Eg. sign the transaction now, but don't submit it on-chain until certain conditions are met.

morkeltry commented 3 years ago

Concerning the risk of users making a mistake leading to stuck transactions - is this risk not greater where users must manipulate transactions outside of metamask for one of the many (and growing more numerous) use cases where control over the transaction after signing is required? (eg private relay, cancel transaction, offline signing and all the many networks being innovated specifically to address flaws in the Ethereum protocl's expected tx flow).

The argument that this restriction 'protects users' grows weaker by the day, unlike the argument that metamask support should not handle queries about mismatched nonces - but in that case, it seems that the user's problem is not prevented or solved, but simply pushed into an area where metamask support won't be bothered by it.

Given that alternative relays (which may intentionally yield different results on the network) must be (and are) supported, why not allow 'no broadcast' as a pseudo-relay - this would need to be explicitly selected by a user, with an option for Metamask to provide warnings. eth_signTransaction can then be limited by settings to only be usable for a configurable list of relays (by default only 'no broadcast'). This would allow users to take responsibility for trusting or not trusting certain networks to hold signed transactions without broadcasting them.

It would mean Metamask would need to give up sole control over nonce management, to be shared with users who have opted-in by requesting a signed transaction, or by explicitly trusting a specific network. But realistically, Metamask does not have this sole control - many users are already managing transaction lifecycle outside of metamask, in ways that could cause the anticipated potential conflicts, but they are doing it using other less reliable tools and workarounds.