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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Identifiers with $ are not recognized #163

Closed
3 tasks done
ardislu opened this issue Jul 8, 2023 · 2 comments · Fixed by #164
Closed
3 tasks done

Identifiers with $ are not recognized #163

ardislu opened this issue Jul 8, 2023 · 2 comments · Fixed by #164

Comments

@ardislu
Copy link
Contributor

ardislu commented Jul 8, 2023

Describe the bug

Minimum code to reproduce the issue:

import { parseAbi } from 'abitype';

const abi = [
  'function _()',
  'function $()',
  'function $_()',
  'function $_a9()',
  'function a9$_()'
];

const result = parseAbi(abi);

Expected result: ABI is parsed successfully.

Actual result: UnknownSignatureError is thrown:

node_modules\abitype\src\human-readable\runtime\utils.ts:148
  throw new UnknownSignatureError({ signature })
        ^
UnknownSignatureError: Unknown signature.

I believe the issue is with the regex to match the function name (functionSignatureRegex in /src/human-readable/runtime/signatures.ts):

const functionSignatureRegex =
  /^function (?<name>[a-zA-Z0-9_]+)\((?<parameters>.*?)\)(?: (?<scope>external|public{1}))?(?: (?<stateMutability>pure|view|nonpayable|payable{1}))?(?: returns\s?\((?<returns>.*?)\))?$/

From the Solidity docs, the regex to match a valid identifier is: [a-zA-Z$_][a-zA-Z0-9$_]*. So it should be a quick fix:

-  /^function (?<name>[a-zA-Z0-9_]+)\((?<parameters>.*?)\)(?: (?<scope>external|public{1}))?(?: (?<stateMutability>pure|view|nonpayable|payable{1}))?(?: returns\s?\((?<returns>.*?)\))?$/
+  /^function (?<name>[a-zA-Z$_][a-zA-Z0-9$_]*)\((?<parameters>.*?)\)(?: (?<scope>external|public{1}))?(?: (?<stateMutability>pure|view|nonpayable|payable{1}))?(?: returns\s?\((?<returns>.*?)\))?$/

I only tested functions, but events, errors, structs, etc. all use the same identifier logic so those regexes should be updated too.

Also wanted to note this ABI works in ethers as expected:

const abi = [
  'function _()',
  'function $()',
  'function $_()',
  'function $_a9()',
  'function a9$_()'
];
const ethers = await import('https://cdn.jsdelivr.net/npm/ethers@6.6.2/+esm');
const interface = new ethers.Interface(abi);
interface.getFunction('$').selector;
// '0xf461e06a'

Lastly for reference, I was able to successfully deploy a smart contract on Sepolia using these identifiers and uploaded it to Etherscan: https://sepolia.etherscan.io/address/0x8ae917bd7ba1bc35ebf7ef1e6ff815565647ab9a#code

Link to Minimal Reproducible Example

No response

Steps To Reproduce

No response

Package Version

0.9.0

TypeScript Version

5.1.6

Anything else?

No response

Validations

@ardislu ardislu changed the title Identifiers with $ or _ are not recognized Identifiers with $ are not recognized Jul 8, 2023
@Raiden1411
Copy link
Collaborator

You are right. Feel free to create a PR to fix this. This does affect also error and event signatures. This should also affect the regex for named parameters

@github-actions
Copy link
Contributor

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest ABIType version. If you have any other comments you can create a new discussion.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2023
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 a pull request may close this issue.

2 participants