Skip to content

chore: add periodic check for spl tokens#8400

Open
sahar-fehri wants to merge 6 commits intomainfrom
fix/add-periodic-checks-for-nonevm-scan
Open

chore: add periodic check for spl tokens#8400
sahar-fehri wants to merge 6 commits intomainfrom
fix/add-periodic-checks-for-nonevm-scan

Conversation

@sahar-fehri
Copy link
Copy Markdown
Contributor

@sahar-fehri sahar-fehri commented Apr 8, 2026

Explanation

Tightens Blockaid bulk token scanning for auto-detected multichain token: assets and adds a periodic re-scan so holdings that later flip to malicious can be dropped without relying on a one-time scan at add time.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Changes token auto-detection behavior to require successful Blockaid scan results and introduces periodic polling that can remove previously stored tokens, which may affect asset visibility and relies on external scan availability.

Overview
Adds a periodic Blockaid re-scan to MultichainAssetsController (now a StaticIntervalPollingController) to re-evaluate stored SPL-style token: assets and automatically ignore/remove any that later become malicious; interval is configurable via blockaidTokenRescanInterval and can be disabled.

Tightens Blockaid filtering for newly auto-detected token: assets to fail closed: tokens are only added when PhishingController:bulkScanTokens returns an explicit non-malicious result, and tokens are dropped when scans error, return empty, omit an address, or when a batch fails.

Updates tests to cover the new fail-closed semantics, batching behavior, and periodic rescan scenarios, and documents the behavior in the package changelog (also removes prior eslint suppression entries for this controller).

Reviewed by Cursor Bugbot for commit 07b8cfa. Bugbot is set up for automated code reviews on this repo. Configure here.

@sahar-fehri sahar-fehri requested review from a team as code owners April 8, 2026 10:06
@sahar-fehri sahar-fehri requested a review from a team as a code owner April 8, 2026 13:00
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 551ed17. Configure here.

* Default interval for re-scanning stored SPL (`token:`) assets with Blockaid.
* Once per day limits API load while still catching tokens reclassified after add.
*/
const DEFAULT_BLOCKAID_TOKEN_RESCAN_INTERVAL_MS = 24 * 60 * 60 * 1000;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

any reason why it's 24h ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think doing once a day is reasonable, unless anyone disagrees. More than that i believe would be a bit overkill


// TODO: make this controller extends StaticIntervalPollingController and update all assetsMetadata once a day.
/** Phishing API allows at most this many token addresses per bulk scan request. */
const BLOCKAID_BULK_TOKEN_SCAN_BATCH_SIZE = 100;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this because of the DynamoDB limit ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The bulkScanTokens in core has already a hardcoded limit of 100, so i think it makes sense for this to not send anything more than that

try {
return parseCaipAssetType(asset).assetNamespace === 'token';
} catch {
return false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we log the error here , this should never throw in theory

return keptTokenAssets.has(asset);
}
} catch {
return false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here , in theory this should never throw and the try catch is not needed , but let's just log it

Copy link
Copy Markdown
Contributor

@salimtb salimtb left a comment

Choose a reason for hiding this comment

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

LGTM

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.

3 participants