Swift SDK Verify API labels non-Scam apps as Scam (no user impact luckily)
Incident Report for WalletConnect
Postmortem

TL:DR: In our Swift SDK versions 1.8.2-1.9.8 we labelled all dapps that were not considered as scam by the verify server as scam.

Impact ******No end-users were affected as wallets don’t respect our scam label.

BitGet, who are in the process of integrating the Verify API into their wallet, was affected but they didn’t release.

Trust who uses Swift and integrated Swift doesn’t show specific UI for isScam so no end-users were affected.

Summary Recently, an issue was identified in our Swift SDK that impacted the way domains are recognized by the Swift Verify client. As an example, the client mistakenly flagged OpenSea as a scam domain. This error stemmed from a bug in our SDK's logic handling.

The error was reported by apps integrating verify API.

To rectify this issue, we have revised the SDK's logic. The updated algorithm now correctly identifies a domain as a scam only if the scam parameter received from the verify server is set to true.

Wallets need to upgrade to Swift SDK version.

After the wallet developer raised the issue we were quick to publish a fix. It however took us ~10h to publish a new version as the 2nd Swift developer who usually approves Swift PRs was on vacation. No other folks were asked for a review until a teammate discovered this and approved. The author of the PR then swiftly released a new version.

Detailed Bug Description: The bug was rooted in the way the SDK interpreted the isScam flag. The logic was designed to treat a nil value for isScam as an indication of the domain not being a scam. However, due to a flaw in this logic, any Boolean value - either true or false - was incorrectly classified as a scam.

5 Whys

********Why were unverified apps flagged as Scams?

Because we had a bug in our logic.

Why did this logic make it into the stable released?

This was not covered by integration or unit tests.

Why was this logic not covered by tests?

We did not prioritize these tests.

Why were these tests not prioritized?

It was acceptable to ship crucial functionality without tests.

Why was it acceptable to ship crucial functionality without tests?

Because we had competing test priorities.

What we could have done better:

We should have escalated this issue immediately and assessed its impact immediately. Luckily no end-users were affected but this would have been a terrible end-user experience.

Another team mate should have immediately been notify for the pull request and the new version should have been published immediately.

Resolution Actions:

  1. Cover this logic in unit tests: Owner @Bartosz Rozwarski
  2. Make sure this logic is covered in all other SDKs @Talha Ali
  3. Audit Swift and other SDKs for appropriate test coverage @Talha Ali @Bartosz Rozwarski
  4. In engineering all-hands we will raise that such issues have to be escalated
Posted Nov 24, 2023 - 11:51 UTC

Resolved
**TL:DR:** In our Swift SDK versions 1.8.2-1.9.8 we labelled all dapps that were not considered as scam by the verify server as scam.

**Impact** ******No end-users were affected as wallets don’t respect our scam label.

BitGet, who are in the process of integrating the Verify API into their wallet, was affected but they didn’t release.

Trust who uses Swift and integrated Swift doesn’t show specific UI for `isScam` so no end-users were affected.

**Summary**
Recently, an issue was identified in our Swift SDK that impacted the way domains are recognized by the Swift Verify client. As an example, the client mistakenly flagged OpenSea as a scam domain. This error stemmed from a bug in our SDK's logic handling.

The error was reported by apps integrating verify API.

To rectify this issue, we have revised the SDK's logic. The updated algorithm now correctly identifies a domain as a scam only if the **`scam`** parameter received from the verify server is set to **`true`**.

Wallets need to upgrade to Swift SDK version.

After the wallet developer raised the issue we were quick to publish a fix. It however took us ~10h to publish a new version as the 2nd Swift developer who usually approves Swift PRs was on vacation. No other folks were asked for a review until a teammate discovered this and approved. The author of the PR then swiftly released a new version.

**Detailed Bug Description:**
The bug was rooted in the way the SDK interpreted the **`isScam`** flag. The logic was designed to treat a **`nil`** value for **`isScam`** as an indication of the domain not being a scam. However, due to a flaw in this logic, any Boolean value - either **`true`** or **`false`** - was incorrectly classified as a scam.

************5 Whys************

*********Why were `unverified` apps flagged as Scams?*

Because we had a bug in our logic.

********Why did this logic make it into the stable released?********

This was not covered by integration or unit tests.

*Why was this logic not covered by tests?*

We did not prioritize these tests.

************************Why were these tests not prioritized?************************

It was acceptable to ship crucial functionality without tests.

******************************************************************Why was it acceptable to ship crucial functionality without tests?******************************************************************

Because we had competing test priorities.

**What we could have done better:**

We should have escalated this issue immediately and assessed its impact immediately. Luckily no end-users were affected but this would have been a terrible end-user experience.

Another team mate should have immediately been notify for the pull request and the new version should have been published immediately.

**Resolution Actions:**

1. Cover this logic in unit tests: Owner @Bartosz Rozwarski
2. Make sure this logic is covered in all other SDKs @Talha Ali
3. Audit Swift and other SDKs for appropriate test coverage @Talha Ali @Bartosz Rozwarski
4. In engineering all-hands we will raise that such issues have to be escalated
Posted Nov 23, 2023 - 11:50 UTC