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

Fix Solana address detection #49

Open
wants to merge 1 commit into
base: prod
Choose a base branch
from
Open

Conversation

vermi321
Copy link

At the moment the TS build doesn't pass.

src/pages/content/components/twitter/tweetHandlers.tsx:116:33 - error TS2488: Type 'true' must have a '[Symbol.iterator]()' method that returns an iterator.

116     for (const solanaAddress of hasSolAddress) {
                                    ~~~~~~~~~~~~~

src/pages/content/components/twitter/tweetHandlers.tsx:126:23 - error TS2339: Property 'length' does not exist on type 'true'.

126     if (hasSolAddress.length == numberOfFalsePositiveSolanaAddresses) {
                          ~~~~~~

The reason for that is the checkForSolAddress function returns boolean while in the code it is expected to return string[] | null. Also the extra check in the checkForSolAddress for A-Z, a-z and 0-9 matches don't do anything extra which isn't already covered by the Solana address regex a few lines above, which makes it simply overcomplicated and it can be inlined just like EVM address matcher.

const solanaAddressRegex = /([1-9A-HJ-NP-Za-km-z]{39,44})/g;
const res = tweetText.match(solanaAddressRegex);
if (!res) return false;
return res.some((str) => regexUpper.test(str) && regexLower.test(str) && regexNumber.test(str));
Copy link
Author

Choose a reason for hiding this comment

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

Filtering out address without characters in any of these 3 categories isn't right, because it would result in false negatives - it is possible to have Solana address without numbers or lower/upper cased letter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant