web3 / web3.js

Collection of comprehensive TypeScript libraries for Interaction with the Ethereum JSON RPC API and utility functions.
https://web3js.org/
Other
19.21k stars 4.92k forks source link

Utils.soliditySha3 types do not like address arrays #2664

Closed rhlsthrm closed 5 years ago

rhlsthrm commented 5 years ago

Description

Typescript linter does not like this type of format w3utils.soliditySha3({ type: 'address[2]', value: [user, recipient] }).

Expected behavior

Should not cause an error.

Actual behavior

Following error is thrown:

Argument of type '{ type: string; value: string[]; }' is not assignable to parameter of type 'Mixed'.
  Type '{ type: string; value: string[]; }' is not assignable to type '{ type: string; value: string; }'.
    Types of property 'value' are incompatible.
      Type 'string[]' is not assignable to type 'string'.ts(2345)

Steps to reproduce the behavior

Error Logs

Gists

Versions

rhlsthrm commented 5 years ago

Looking into the index.d.ts for Utils, I see

export type Mixed =
    | string
    | number
    | BN
    | {
          type: string;
          value: string;
      }
    | {
          t: string;
          v: string | BN | number;
      }
    | boolean;

This typedef also seems wrong, why does the { t, v } syntax support numbers and BN but { type, value } does not?

princesinha19 commented 5 years ago

@rhlsthrm Why are you using value: [user, recipient]? Can you explain a bit regarding use case. Value field can only be a nymber, or a BN, or string.

rhlsthrm commented 5 years ago

We are trying to hash an array of addresses. Solidity's keccak256 function handles this perfectly well. This is an example in solidity:

function _verifySig (
    address[2] user, // [user, recipient]
    uint256[2] weiBalances, // [hub, user]
    uint256[2] tokenBalances, // [hub, user]
    uint256[4] pendingWeiUpdates, // [hubDeposit, hubWithdrawal, userDeposit, userWithdrawal]
    uint256[4] pendingTokenUpdates, // [hubDeposit, hubWithdrawal, userDeposit, userWithdrawal]
    uint256[2] txCount, // [global, onchain] persisted onchain even when empty
    bytes32 threadRoot,
    uint256 threadCount,
    uint256 timeout,
    string sigHub,
    string sigUser,
    bool[2] checks // [checkHubSig?, checkUserSig?]
) internal view {
    require(user[0] != hub, "user can not be hub");
    require(user[0] != address(this), "user can not be channel manager");

    // prepare state hash to check hub sig
    bytes32 state = keccak256(
        abi.encodePacked(
            address(this),
            user, // [user, recipient]
            weiBalances, // [hub, user]
            tokenBalances, // [hub, user]
            pendingWeiUpdates, // [hubDeposit, hubWithdrawal, userDeposit, userWithdrawal]
            pendingTokenUpdates, // [hubDeposit, hubWithdrawal, userDeposit, userWithdrawal]
            txCount, // persisted onchain even when empty
            threadRoot,
            threadCount,
            timeout
        )
    );

    if (checks[0]) {
        require(hub == ECTools.recoverSigner(state, sigHub), "hub signature invalid");
    }

    if (checks[1]) {
        require(user[0] == ECTools.recoverSigner(state, sigUser), "user signature invalid");
    }
}

This is essentially the onchain functionality we are trying to replicate off chain. Also, this has always worked in web3's soliditySha3 function.

princesinha19 commented 5 years ago

This is working with soliditySha3 too. I tested this with an example: web3.utils.soliditySha3({ t: "uint256", v: ["0x407D73d8a49eeb85D32Cf465507dd71d507100c1", "0x247149f8b797DEA444F7f150259D678b13A565b1","0x407D73d8a49eeb85D32Cf465507dd71d507100c1"] }); And, this is working fine. In types you have to specify uint256, bytes or other type as specified in documentation. https://web3js.readthedocs.io/en/1.0/web3-utils.html#soliditysha3

rhlsthrm commented 5 years ago

Great, thanks. I believe that the code is probably working, as it has before. Good to know I don't need to specify it is an array, as I did in my example i.e. { t: "uint256[2]", v: ... }.

However my issue is with the typescript typings. From your example, it still doesn't look like the type in the index.d.ts accepts an array:

export type Mixed =
    | string
    | number
    | BN
    | {
          type: string;
          value: string;
      }
    | {
          t: string;
          v: string | BN | number;
      }
    | boolean;

I've worked around it by using // @ts-ignore but ideally I would not do this.

princesinha19 commented 5 years ago

@rhlsthrm Why you want the type as the array? value is supporting array so, you can insert any number of addresses in the value field.

rhlsthrm commented 5 years ago

It appears the error isn't showing up anymore, not sure what happened. Thanks for the help, I'll open if I see something again.