MetaMask / metamask-mobile

Mobile web browser providing access to websites that use the Ethereum blockchain
https://metamask.io
Other
2.18k stars 1.12k forks source link

[Bug]: Cannot handle URI Scheme with Metadata, Value and Bytecode, ERC-681 #8308

Open 0xPuddi opened 10 months ago

0xPuddi commented 10 months ago

Describe the bug

The QR can be generated with any library you wish, the Deep Link String is:

ethereum:0x4758aCF2d393A0f011C603eAfC9B4769322b2a94@43113/BetGame?bytes32=0x3863e63da67d85dcffede3cace2ebcbb39e67df5f6fdaf6437319cbe68cc3b94&bytes32=0x0fbf1ff35ca8785e1812f7745b1332b4b9b3c64ccd769f19869170beb4e4f513&value=1.0e15

The contract is deployed (0x4758aCF2d393A0f011C603eAfC9B4769322b2a94) on the Avalanche Fuji Testnet (ID: 43113) and it has a BetGame function that is payable and that takes two bytes32 as input. Function definition:

/**
* BetGame accetps and registers a player's game bet
*/
function BetGame(bytes32 gameId, bytes32 playerId) external payable minBet returns (bool) {
address caller = _msgSender();
uint256 betAmount = msg.value;

    // if firts game make him start it
    if (!getActiveGame(gameId)) {
        emit GameStarted(gameId, playerId, caller, betAmount, block.timestamp);
    }

    // Already in
    if (playerAddress[playerId] != address(0)) {
        revert BetAlreadyPlaced(caller, playerGameBalance[playerId][caller]);
    }

    gamePlayersIds[gameId].push(playerId);
    gameBalance[gameId] += betAmount;
    playerAddress[playerId] = caller;
    playerGameBalance[playerId][caller] = betAmount;
    emit PlayerBet(gameId, playerId, caller, betAmount, block.timestamp);
    return true;
}

/**
 * getActiveGame reutrns true is a game is active
 */
function getActiveGame(bytes32 gameId) public view returns (bool) {
    if (gamePlayersIds[gameId].length > 0) {
        return true;
    }

    return false;
}

The MetaMask Wallet reads the URL, and creates a transaction, yet the gas is miscalculated. Miscalculation of gas is probably a consequence that when you try to send the transaction it fails.

The error is a "Transaction error Internal JSON-RPC error." I tried changing also the RPC endpoint multiple times with different providers and I get the same error. Sending native coin around works.

If it can help: The value to send is correct, the chain is correct, the only problem there is is that the calldata with the function signature and input data is wrong, or not even there at all.

Expected behavior

  1. Read the QR Code

  2. Generate a transaction:

On the cainId: 43113 To: 0x4758aCF2d393A0f011C603eAfC9B4769322b2a94 With function signature: BetGame With Input data: bytes32=0x3863e63da67d85dcffede3cace2ebcbb39e67df5f6fdaf6437319cbe68cc3b94 and bytes32=0x0fbf1ff35ca8785e1812f7745b1332b4b9b3c64ccd769f19869170beb4e4f513 And value sent of 1.0e15

  1. Send the transaction and notify on confirmation.

Screenshots/Recordings

No response

Steps to reproduce

Use the Deep Link string:

ethereum:0x4758aCF2d393A0f011C603eAfC9B4769322b2a94@43113/BetGame?bytes32=0x3863e63da67d85dcffede3cace2ebcbb39e67df5f6fdaf6437319cbe68cc3b94&bytes32=0x0fbf1ff35ca8785e1812f7745b1332b4b9b3c64ccd769f19869170beb4e4f513&value=1.0e15

Choose your favourite QR code generation method (Attached a QR code generated from Brave Browser with QR code feature. The String is the Deep Link string just mentioned above 33ca327c520c47e0a0ca681fa0bb0d41720e35cc ).

Have some gas on Avalanche Fuji Testner.

Generate the QR.

Scan the QR with MetaMask Mobile.

Error messages or log output

Internal JSON-RCP Error popup.

Version

MetaMask v7.12.3 (1230)

Build type

None

Device

Iphone XR

Operating system

iOS

Additional context

No response

Severity

Critical to all MetaMask users and developers.

The problem is that some apps functionality might not work as expected, since the ERC is finalised and well accepted developers should be able to interact using the standard without any problems or drawbacks.

DanielTech21 commented 10 months ago

Hi @Puddi1 Thanks for bringing this to our attention. I have assigned this to the right team to investigate it more.

christopherferreira9 commented 7 months ago

Hi @0xPuddi ! This has just caught our attention, can you please check if the issue persists with the latest version of the MetaMask mobile wallet?

0xPuddi commented 7 months ago

Hey @christopherferreira9 thanks for your help and time. It is yes working, meaning that it identifies the contract, the chain and the value amount to send and there is no RPC-ERROR. Yet it is not including any data bytes, meaning that if you need to make a contract call you won't be able to do so, as it is able to send just eth, and if there is no eth (value) to send over (think about making a contract call without ether) it will prompt you to add some before sending the transaction, like if it was trying to make a native send.

In the example ethereum:0x4758aCF2d393A0f011C603eAfC9B4769322b2a94@43113/BetGame?bytes32=0x3863e63da67d85dcffede3cace2ebcbb39e67df5f6fdaf6437319cbe68cc3b94&bytes32=0x0fbf1ff35ca8785e1812f7745b1332b4b9b3c64ccd769f19869170beb4e4f513&value=1.0e15

It is like it's getting this right: ethereum:0x4758aCF2d393A0f011C603eAfC9B4769322b2a94@43113 this wrong (missing): /BetGamebytes32=0x3863e63da67d85dcffede3cace2ebcbb39e67df5f6fdaf6437319cbe68cc3b94&bytes32=0x0fbf1ff35ca8785e1812f7745b1332b4b9b3c64ccd769f19869170beb4e4f513 this right: &value=1.0e15

For any parsing question refer to the approved EIP standard

Thank you!

christopherferreira9 commented 7 months ago

Hi @gauthierpetetin ! What should be the responsible team to handle this topic since the deeplink part is being properly handled? cc: @andreahaku

gauthierpetetin commented 6 months ago

Hi @christopherferreira9 , bschorchit reviewed it and assigned it to Transactions team.

forest-diggs-consensys commented 6 months ago

Hey @bschorchit - could you give some context on why this falls under Transactions?

bschorchit commented 6 months ago

Hey @forest-diggs-consensys, based on the original report and since @christopherferreira9 mentioned that the deeplink part is working correctly, it seems that the problem is around how we're calculating gas for the transaction proposed through the deeplink or even how we're not persisting the correct calldata for it.

The MetaMask Wallet reads the URL, and creates a transaction, yet the gas is miscalculated. The value to send is correct, the chain is correct, the only problem there is is that the calldata with the function signature and input data is wrong, or not even there at all.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has not had recent activity in the last 90 days. It will be closed in 7 days. Thank you for your contributions.