fix(security): replace simulated risk checks with honest unverified results - #939
Open
amossamuel851-tech wants to merge 2 commits into
Open
Conversation
…esults Remove the fabricated address/transaction risk scores derived from hashes and placeholder known-scam lists. Address and transaction checks now report verified:false with an explicit "unable to verify" result whenever no real screening provider produced a score, and the UI surfaces an unverified state instead of presenting a guessed score as fact. Wallet similarity and known-scam-contract checks now operate on the real blocklist, and the placeholder isSuspiciousAddress recipient anomaly is removed. 🤖 Generated with Codebuff Co-Authored-By: Codebuff <[email protected]>
|
@amossamuel851-tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
nanaf6203-bit
approved these changes
Aug 26, 2026
nanaf6203-bit
left a comment
Contributor
There was a problem hiding this comment.
Nice work, thanks for getting this over the line!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #818
Replaces the fabricated wallet-security layer in
src/utils/security/with honest behavior: hex-derived risk scores, always-false scam checks, and a placeholder suspicious-address anomaly are gone, and every surface now reportsverified: falsewith an explicit "unable to verify" result when no real screening provider produced a score. The transaction-confirmation UI surfaces an explicit "Not verified" state instead of presenting a guessed number as a real risk signal.Why
The security layer presented fiction as fact.
simulateAddressRiskCheckderived a deterministic score from the address's hex characters (parseInt(addressHash.slice(0, 8), 16) % 100),hasSimilarityToKnownAddressesalways returnedfalse,isKnownScamContractchecked an empty hardcoded list, andisSuspiciousAddresswas a permanentfalseplaceholder that nonetheless fed a "flagged in security database" anomaly intoTransactionMonitor. Users saw a "73/100 risk" badge and warnings that no screening had actually produced, creating false confidence right before transfer approval. The failure was silent by design: every call returned plausible-looking values, so nothing flagged the stubs.Per the issue's out-of-scope note, integrating a real third-party risk API is a separate service decision. The correct, shippable behavior is to stop the simulation: report the honest unverified state and define the fallback contract, which is exactly what this PR does. Where real logic already existed (the
RISKY_WALLETSblocklist), the placeholder checks were wired to it instead of being deleted.What was built
src/utils/security/blockchainSecurity.tssimulateAddressRiskCheckandsimulateTransactionRiskCheckdeleted.AddressRiskScore/TransactionRisk/validateTransactiongain averifiedfield.checkAddressRiskreturns theunable_to_verifydefault (verified: false) whenever the proxy is unconfigured, unreachable, or returns no numeric score.checkTransactionRiskno longer derives a score from the hash — it returns the honest default. Fixed the pre-existinglogger is not definedreference (the file calledlogger.errorwithout importing it).src/utils/security/walletValidator.tsKNOWN_SCAM_ADDRESSESlist containing a fake "example scam address".hasSimilarityToKnownAddressesnow computes real edit-distance similarity against theRISKY_WALLETSblocklist (>0.8 threshold).isKnownScamContractnow checks the blocklist directly. TheRISKY_WALLETSentries are real addresses (e.g. the null address,0x000...0001).src/utils/security/transactionMonitor.tsisSuspiciousAddressplaceholder deleted, along with the fabricated "suspicious_recipient" anomaly that claimed "Address flagged in security database" — a claim no real check backed.src/hooks/useSecurity.tsTransactionValidationgainsriskVerified.validateTransactionpopulates it from the blockchain service'sverifiedflag and pushes an explicit warning ("Address risk screening unavailable - address could not be verified") when screening did not run.src/components/TransactionConfirmation.tsxriskVerifiedis false, the Security Assessment panel shows a gray "Not verified" badge, a neutral icon, and the message "Address risk screening is unavailable, so no risk score is available. Verify the recipient address manually." — no fabricated number or color-coded risk level.src/utils/security/__tests__/blockchainSecurity.test.ts@/utils/logger(fixing the pre-existinglogger is not definedcrash), and asserts the new honest behavior — unverified defaults on proxy failure/non-ok/missing-score/verified:false, verified results when a real score arrives,validateTransaction.verifiedpropagation, and that the simulate methods no longer exist.src/utils/security/__tests__/walletValidator.test.ts@/utils/logger(same pre-existing crash), added blocklist state snapshot/restore for test isolation, and rewrote the placeholder tests:hasSimilarityToKnownAddressesreturnstruefor a similar address,isKnownScamContractreturnstruefor the null address.src/utils/security/__tests__/transactionMonitor.test.tsisSuspiciousAddressno longer exists and no "flagged in security database" anomaly is ever fabricated, while normal transaction/metrics behavior still works.The tests pin the acceptance criteria directly: the "no fabricated score" tests would fail against the old code (which returned deterministic values and exposed the simulate methods) and pass against the new behavior.
Integration changes outside
src/utils/security/src/hooks/useSecurity.ts—TransactionValidation.riskVerifiedadded; wired from the blockchain service'sverifiedflag.src/components/TransactionConfirmation.tsx— explicit "Not verified" UI state replaces the unconditional risk badge when screening did not run.Acceptance criteria coverage
simulateAddressRiskCheckis either backed by a real check or removed. (blockchainSecurity.ts— both simulate methods deleted;checkAddressRisk/checkTransactionRiskreturn the honest default; testdoes not expose simulateAddressRiskCheck or simulateTransactionRiskCheckandshould return an unverified default (no fabricated score from the hash))isKnownScamContract/hasSimilarityToKnownAddresses/isSuspiciousAddresseither implement real screening or are removed, and no UI element claims to have performed those checks. (walletValidator.ts— real blocklist-backed checks;transactionMonitor.ts—isSuspiciousAddressand its fabricated anomaly removed; testsshould return true for addresses similar to a known risky address,should return true for null address (in blocklist),no longer exposes the placeholder isSuspiciousAddress check)TransactionConfirmation.tsx— "Not verified" badge + explanatory message;useSecurity.ts—riskVerifiedflag and screening-unavailable warning)blockchainSecurity.test.ts,walletValidator.test.ts, newtransactionMonitor.test.ts— 60 new/changed tests, all passing)npm run typecheck,npm test, andnpm run lintpass — blocked by pre-existing failures onmain, none caused by this PR (see Test plan for evidence).Test plan
npx jest src/utils/security/__tests__/{blockchainSecurity,walletValidator,transactionMonitor}.test.ts— 87/90 passing; the 3 failures (trim, ENS message wording, 0.0.0.0 domain) are pre-existing and unrelated, confirmed by stash comparison (they fail identically without my changes)npm test— 883/1019 passing vs baseline 820/959 without my changes: +63 net passing tests, 0 new failures (the 136 failures are pre-existing onmain, e.g. missingdefineChainin the viem mock; my changes actually fix 3 pre-existinglogger is not definedcrashes in the blockchain suite)npm run typecheck— 36 errors, all pre-existing in 8 untouched files (identical count with and without my changes, verified by stash)npx eslint <touched files>— 0 new errors from this PR (the +3jsdoc/require-jsdocerrors my new code initially introduced were fixed; remaining 30 errors in these files are pre-existing in untouched code). Repo-widenpm run lintis broken onmainitself:eslint.config.mjsimportseslint-plugin-jsdoc, which is declared nowhere in the dependency tree (~3555 baseline violations). Test-fileas anywarnings match the project's documented test-only convention (docs/as-any-survivors.md).npx next build— blocked by pre-existingModule not found: Can't resolve 'dns'/'fs'/'net'(ioredis/redis-client bundling) in untouched files; none of this PR's files appear in the failure traceEnv vars / Notes
No new environment variables. The
verifiedflag is additive and backward-compatible: consumers that ignore it see the sameriskScoreshape, and theunable_to_verifydefault was already the documented fallback for failed checks. The honest fallback contract (verified: false+unable_to_verifylabels) is the integration seam a future real screening provider plugs into.