Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display boolean values when signing typed data #10048

Merged
merged 3 commits into from Dec 11, 2020
Merged

Display boolean values when signing typed data #10048

merged 3 commits into from Dec 11, 2020

Conversation

nlordell
Copy link
Contributor

@nlordell nlordell commented Dec 10, 2020

Fixes: #4639.

Explanation: The confirmation popup when signing typed data was not correctly displaying boolean fields. This is because booleans have special meaning in JSX and don't render their string representation (source). The fix is simple, just toString() the values being displayed.

Manual testing steps:

  • Load sample code included below 馃憞.

  • Check to see that the values are correctly displayed (especially noting the bool value which is displayed now)
    image

  • Profit 馃挾

index.html
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>Sign Typed Data Test</title>
    <script src="./index.js"></script>
  </head>
  <body>
    <button id="sign">eth_signTypedData</button>
  </body>
</html>
index.js
import { ethers, BigNumber } from "ethers";

async function main() {
  await window.ethereum.enable();

  const provider = new ethers.providers.Web3Provider(window.ethereum);
  const signer = provider.getSigner();
  const account = await signer.getAddress();

  document.querySelector("#sign").addEventListener("click", async () => {
    const signature = await provider.send("eth_signTypedData_v4", [
      account.toLowerCase(),
      JSON.stringify({
        types: {
          EIP712Domain: [
            { name: "name", type: "string" },
          ],
          Data: [
            { name: "bytes2", type: "bytes2" },
            { name: "uint32", type: "uint32" },
            { name: "bool", type: "bool" },
            { name: "bytes", type: "bytes" },
            { name: "string", type: "string" },
            { name: "nested", type: "Inner[]" },
          ],
          Inner: [
            { name: "int8", type: "int8" },
            { name: "uint128", type: "uint128" },
            { name: "address", type: "address" },
          ],
        },
        primaryType: "Data",
        domain: { name: "Test" },
        message: {
          bytes2: "0x1337",
          uint32: "0xbadc0de",
          bool: true,
          bytes: "0x010203040506070809",
          string: "Hello MetaMask!",
          nested: [{
            int8: -42,
            uint128: BigNumber.from("12384761928764192812341324214").toHexString(),
            address: account,
          }],
        },
      })
    ]);

    console.log(signature);
  });
}

main()
  .then(() => console.log("done"))
  .catch((err) => console.error(err));

@nlordell nlordell requested a review from a team as a code owner December 10, 2020 22:23
@github-actions
Copy link
Contributor

github-actions bot commented Dec 10, 2020

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@nlordell
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Great work, @nlordell

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@brad-decker brad-decker merged commit fcf75d6 into MetaMask:develop Dec 11, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should display values of EIP712 typed boolean fields in the signing prompt
3 participants