nucypher / taco-web

🌮 A TypeScript client for TACo (Threshold Access Control)
https://docs.threshold.network/app-development/threshold-access-control-tac
GNU General Public License v3.0
14 stars 20 forks source link

Add "address payable" to schema validation #515

Closed manumonti closed 2 months ago

manumonti commented 2 months ago

Type of PR:

Required reviews:

What this does: This change will add the solidity type address payable to the list of types that are supported by schema validation.

Why it's needed: When using a contract-type condition with address payable as the internal type, an error is raised.

For instance, the following function ABI

  const funcABI: conditions.base.contract.FunctionAbiProps = {
    inputs: [
      {
        internalType: 'bytes32',
        name: 'node',
        type: 'bytes32',
      },
    ],
    name: 'addr',
    outputs: [
      {
        internalType: 'address payable', // <- Problem here
        name: '',
        type: 'address',
      },
    ],
    stateMutability: 'view',
    type: 'function',
  };

will return the following error:

/Users/manumonti/Projects/nucypher/taco-web/packages/taco/src/conditions/condition.ts:19
      throw new Error(ERR_INVALID_CONDITION(error));
            ^
Error: Invalid condition: [{"received":"address payable","code":"invalid_enum_value","options":["bool","string","address","bytes1","bytes2","bytes3","bytes4","bytes5","bytes6","bytes7","bytes8","bytes9","bytes10","bytes11","bytes12","bytes13","bytes14","bytes15","bytes16","bytes17","bytes18","bytes19","bytes20","bytes21","bytes22","bytes23","bytes24","bytes25","bytes26","bytes27","bytes28","bytes29","bytes30","bytes31","bytes32","bytes","uint8","uint16","uint24","uint32","uint40","uint48","uint56","uint64","uint72","uint80","uint88","uint96","uint104","uint112","uint120","uint128","uint136","uint144","uint152","uint160","uint168","uint176","uint184","uint192","uint200","uint208","uint216","uint224","uint232","uint240","uint248","uint256","int8","int16","int24","int32","int40","int48","int56","int64","int72","int80","int88","int96","int104","int112","int120","int128","int136","int144","int152","int160","int168","int176","int184","int192","int200","int208","int216","int224","int232","int240","int248","int256"],"path":["functionAbi","outputs",0,"internalType"],"message":"Invalid enum value. Expected 'bool' | 'string' | 'address' | 'bytes1' | 'bytes2' | 'bytes3' | 'bytes4' | 'bytes5' | 'bytes6' | 'bytes7' | 'bytes8' | 'bytes9' | 'bytes10' | 'bytes11' | 'bytes12' | 'bytes13' | 'bytes14' | 'bytes15' | 'bytes16' | 'bytes17' | 'bytes18' | 'bytes19' | 'bytes20' | 'bytes21' | 'bytes22' | 'bytes23' | 'bytes24' | 'bytes25' | 'bytes26' | 'bytes27' | 'bytes28' | 'bytes29' | 'bytes30' | 'bytes31' | 'bytes32' | 'bytes' | 'uint8' | 'uint16' | 'uint24' | 'uint32' | 'uint40' | 'uint48' | 'uint56' | 'uint64' | 'uint72' | 'uint80' | 'uint88' | 'uint96' | 'uint104' | 'uint112' | 'uint120' | 'uint128' | 'uint136' | 'uint144' | 'uint152' | 'uint160' | 'uint168' | 'uint176' | 'uint184' | 'uint192' | 'uint200' | 'uint208' | 'uint216' | 'uint224' | 'uint232' | 'uint240' | 'uint248' | 'uint256' | 'int8' | 'int16' | 'int24' | 'int32' | 'int40' | 'int48' | 'int56' | 'int64' | 'int72' | 'int80' | 'int88' | 'int96' | 'int104' | 'int112' | 'int120' | 'int128' | 'int136' | 'int144' | 'int152' | 'int160' | 'int168' | 'int176' | 'int184' | 'int192' | 'int200' | 'int208' | 'int216' | 'int224' | 'int232' | 'int240' | 'int248' | 'int256', received 'address payable'"}]
    at new Condition (/Users/manumonti/Projects/nucypher/taco-web/packages/taco/src/conditions/condition.ts:19:13)
    at new ContractCondition (/Users/manumonti/Projects/nucypher/taco-web/packages/taco/src/conditions/base/contract.ts:103:5)
    at encryptToBytes (/Users/manumonti/Projects/nucypher/taco-web/examples/taco/nodejs/src/index.ts:72:21)
    at async runExample (/Users/manumonti/Projects/nucypher/taco-web/examples/taco/nodejs/src/index.ts:128:26)
netlify[bot] commented 2 months ago

Deploy Preview for taco-nft-demo canceled.

Name Link
Latest commit fce5be32a6afe7812ed15cc95a77ccf34093c26c
Latest deploy log https://app.netlify.com/sites/taco-nft-demo/deploys/662b5826bb3ccb0008b81848
netlify[bot] commented 2 months ago

Deploy Preview for taco-demo canceled.

Name Link
Latest commit fce5be32a6afe7812ed15cc95a77ccf34093c26c
Latest deploy log https://app.netlify.com/sites/taco-demo/deploys/662b5826fa078800084c3091
derekpierre commented 2 months ago

Given that conditions should only use view/read-only functions does it make sense for an address parameter to be address payable? (cc @vzotova , @cygnusv )

manumonti commented 2 months ago

Given that conditions should only use view/read-only functions does it make sense for an address parameter to be address payable? (cc @vzotova , @cygnusv )

I can't give you an answer, but just to provide some context, the function ABI in the PR message comes from this function:

https://github.com/ensdomains/ens-contracts/blob/c15e4afd5f1e886094459e5e74d21dd080704443/contracts/resolvers/profiles/AddrResolver.sol#L35-L43

derekpierre commented 2 months ago

Oh I see, the return type is address payable - 👍 . Thanks for the link.