Skip to content

Commit

Permalink
Make the PhishingController test synchronous again
Browse files Browse the repository at this point in the history
The `test` method for the PhishingController was recently made
asynchronous so that it could update the phishing configuration if
necessary. We found that in practice this was difficult to use,
especially when handling multiple simultanous tests.

The `test` method has been made synchronous again. Instead we can check
whether the config is out of date using the new `isOutOfDate` method,
and explicitly update the configuration if required.
  • Loading branch information
Gudahtt committed Oct 7, 2022
1 parent 0b03d58 commit 73a99a9
Showing 1 changed file with 6 additions and 13 deletions.
19 changes: 6 additions & 13 deletions src/third-party/PhishingController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,28 +149,21 @@ export class PhishingController extends BaseController<
}

/**
* Calls this.updatePhishingLists if this.refreshInterval has passed since last this.lastFetched.
* Determine if an update to the phishing configuration is needed.
*
* @returns Promise<void> when finished fetching phishing lists or when fetching in not necessary.
* @returns Whether an update is needed
*/
private async fetchIfNecessary(): Promise<void> {
const outOfDate =
Date.now() - this.lastFetched >= this.config.refreshInterval;

if (outOfDate) {
await this.updatePhishingLists();
}
isOutOfDate() {
return Date.now() - this.lastFetched >= this.config.refreshInterval;
}

/**
* Determines if a given origin is unapproved.
*
* @param origin - Domain origin of a website.
* @returns Promise<EthPhishingDetectResult> Whether the origin is an unapproved origin.
* @returns Whether the origin is an unapproved origin.
*/
async test(origin: string): Promise<EthPhishingDetectResult> {
await this.fetchIfNecessary();

test(origin: string): EthPhishingDetectResult {
const punycodeOrigin = toASCII(origin);
if (this.state.whitelist.indexOf(punycodeOrigin) !== -1) {
return { result: false, type: 'all' }; // Same as whitelisted match returned by detector.check(...).
Expand Down

0 comments on commit 73a99a9

Please sign in to comment.