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: handle projector crash on invalid name #1089

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

mkazlauskas
Copy link
Member

@mkazlauskas mkazlauskas commented Feb 8, 2024

Context

InvalidStringError: Invalid string: "Invalid handle 0|0" when projecting handles on mainnet

Proposed Solution

  • Update handle validation regular expression to be the same as used by ada handle
  • Allow '0|0' handle as an exception as it exists on mainnet
  • Ignore invalid handles instead of crashing the projector

Important Changes Introduced

  • Update handles minted in e2e to be lowercase, because on-chain handle asset names are always lowercase

Copy link

github-actions bot commented Feb 8, 2024

Standard DiffPost

This PR would generate the following kubectl diff:

Preview
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-backend /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-backend
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:37.208020479 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:37.208020479 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "135"
   creationTimestamp: "2023-08-11T18:16:12Z"
-  generation: 135
+  generation: 136
   labels:
     app: backend
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-backend
@@ -89,7 +89,7 @@
           value: http://dev-preview-cardano-stack-metadata.dev-preview.svc.cluster.local
         - name: USE_KORA_LABS
           value: "true"
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-blockfrost-worker /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-blockfrost-worker
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-blockfrost-worker	2024-02-09 09:54:37.416020521 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-blockfrost-worker	2024-02-09 09:54:37.416020521 +0000
@@ -8,7 +8,7 @@
   labels:
     app: blockfrost-worker
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-blockfrost-worker
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-coingecko-proxy /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-coingecko-proxy
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:37.628020563 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:37.628020563 +0000
@@ -8,7 +8,7 @@
   labels:
     app: coingecko-proxy
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-coingecko-proxy
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-projector /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-projector
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-projector	2024-02-09 09:54:37.828020603 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-projector	2024-02-09 09:54:37.828020603 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "135"
   creationTimestamp: "2023-08-11T18:16:13Z"
-  generation: 135
+  generation: 136
   labels:
     app: handle-projector
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-handle-projector
@@ -73,7 +73,7 @@
               name: handle-owner-user.dev-preview-dbsync-db.credentials.postgresql.acid.zalan.do
         - name: PROJECTION_NAMES
           value: handle
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-provider /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-provider
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-provider	2024-02-09 09:54:38.028020641 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-handle-provider	2024-02-09 09:54:38.028020641 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "102"
   creationTimestamp: "2023-08-11T18:16:13Z"
-  generation: 102
+  generation: 103
   labels:
     app: handle-provider
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-handle-provider
@@ -81,7 +81,7 @@
           value: handle
         - name: USE_KORA_LABS
           value: "true"
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-pg-boss-worker /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-pg-boss-worker
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-pg-boss-worker	2024-02-09 09:54:38.232020682 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-pg-boss-worker	2024-02-09 09:54:38.232020682 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "30"
   creationTimestamp: "2024-01-11T10:37:15Z"
-  generation: 30
+  generation: 31
   labels:
     app: pg-boss-worker
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-pg-boss-worker
@@ -97,7 +97,7 @@
           value: https://smash.cardano-mainnet.iohk.io/api/v1
         - name: STAKE_POOL_PROVIDER_URL
           value: http://dev-preview-cardanojs-backend.dev-preview.svc.cluster.local
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-projector /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-projector
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-projector	2024-02-09 09:54:38.436020724 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-projector	2024-02-09 09:54:38.436020724 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "30"
   creationTimestamp: "2024-01-11T10:37:15Z"
-  generation: 30
+  generation: 31
   labels:
     app: stake-pool-projector
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-stake-pool-projector
@@ -73,7 +73,7 @@
               name: stakepool-owner-user.dev-preview-dbsync-db.credentials.postgresql.acid.zalan.do
         - name: PROJECTION_NAMES
           value: stake-pool,stake-pool-metadata-job,stake-pool-metrics-job,stake-pool-rewards-job
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-provider /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-provider
--- /tmp/LIVE-943059871/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-provider	2024-02-09 09:54:38.640020765 +0000
+++ /tmp/MERGED-928726288/apps.v1.Deployment.dev-preview.dev-preview-cardanojs-stake-pool-provider	2024-02-09 09:54:38.640020765 +0000
@@ -4,11 +4,11 @@
   annotations:
     deployment.kubernetes.io/revision: "31"
   creationTimestamp: "2024-01-10T17:49:47Z"
-  generation: 31
+  generation: 32
   labels:
     app: stake-pool-provider
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-stake-pool-provider
@@ -83,7 +83,7 @@
           value: http://dev-preview-cardano-stack-metadata.dev-preview.svc.cluster.local
         - name: USE_TYPEORM_STAKE_POOL_PROVIDER
           value: "true"
-        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:pb6kyjvxnmhkg2gzmzdh6x25axhxn3i1
+        image: 926093910549.dkr.ecr.us-east-1.amazonaws.com/cardano-services:3jlavarjp164aaakwffydsz05mllkkqy
         imagePullPolicy: IfNotPresent
         livenessProbe:
           failureThreshold: 3
diff -u -N /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-backend-monitor /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-backend-monitor
--- /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-backend-monitor	2024-02-09 09:54:38.840020803 +0000
+++ /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-backend-monitor	2024-02-09 09:54:38.840020803 +0000
@@ -5,7 +5,7 @@
   generation: 2
   labels:
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     instance: primary
   name: lace-backend-monitor
   namespace: dev-preview
diff -u -N /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-handle-provider-monitor /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-handle-provider-monitor
--- /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-handle-provider-monitor	2024-02-09 09:54:39.044020843 +0000
+++ /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-handle-provider-monitor	2024-02-09 09:54:39.044020843 +0000
@@ -5,7 +5,7 @@
   generation: 2
   labels:
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     instance: primary
   name: lace-handle-provider-monitor
   namespace: dev-preview
diff -u -N /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-stake-pool-provider-monitor /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-stake-pool-provider-monitor
--- /tmp/LIVE-943059871/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-stake-pool-provider-monitor	2024-02-09 09:54:39.244020884 +0000
+++ /tmp/MERGED-928726288/monitoring.coreos.com.v1.ServiceMonitor.dev-preview.lace-stake-pool-provider-monitor	2024-02-09 09:54:39.244020884 +0000
@@ -5,7 +5,7 @@
   generation: 2
   labels:
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     instance: primary
   name: lace-stake-pool-provider-monitor
   namespace: dev-preview
diff -u -N /tmp/LIVE-943059871/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-backend /tmp/MERGED-928726288/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-backend
--- /tmp/LIVE-943059871/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:39.452020904 +0000
+++ /tmp/MERGED-928726288/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:39.452020904 +0000
@@ -19,7 +19,7 @@
   labels:
     app: backend
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-backend
diff -u -N /tmp/LIVE-943059871/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-coingecko-proxy /tmp/MERGED-928726288/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-coingecko-proxy
--- /tmp/LIVE-943059871/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:39.652020898 +0000
+++ /tmp/MERGED-928726288/networking.k8s.io.v1.Ingress.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:39.652020898 +0000
@@ -16,7 +16,7 @@
   labels:
     app: coingecko-proxy
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-coingecko-proxy
diff -u -N /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-backend /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-backend
--- /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:36.408020700 +0000
+++ /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-backend	2024-02-09 09:54:36.408020700 +0000
@@ -5,7 +5,7 @@
   labels:
     app: backend
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-backend
diff -u -N /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-coingecko-proxy /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-coingecko-proxy
--- /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:36.604020433 +0000
+++ /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-coingecko-proxy	2024-02-09 09:54:36.604020433 +0000
@@ -5,7 +5,7 @@
   labels:
     app: coingecko-proxy
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-coingecko-proxy
diff -u -N /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-handle-provider /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-handle-provider
--- /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-handle-provider	2024-02-09 09:54:36.808020401 +0000
+++ /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-handle-provider	2024-02-09 09:54:36.808020401 +0000
@@ -5,7 +5,7 @@
   labels:
     app: handle-provider
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-handle-provider
diff -u -N /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-stake-pool-provider /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-stake-pool-provider
--- /tmp/LIVE-943059871/v1.Service.dev-preview.dev-preview-cardanojs-stake-pool-provider	2024-02-09 09:54:37.000020437 +0000
+++ /tmp/MERGED-928726288/v1.Service.dev-preview.dev-preview-cardanojs-stake-pool-provider	2024-02-09 09:54:37.000020437 +0000
@@ -5,7 +5,7 @@
   labels:
     app: stake-pool-provider
     app.kubernetes.io/managed-by: std-kubectl
-    app.kubernetes.io/version: 6eb7c78995b24677652a9dbdfccd55cfc989b0ab
+    app.kubernetes.io/version: 26db65648e0093b8a7b7d055cd0442bca2781580
     network: preview
     release: dev-preview-cardanojs
   name: dev-preview-cardanojs-stake-pool-provider

@@ -29,43 +30,45 @@ const project = (tx: Cardano.OnChainTx) =>
)
);

const createCip25HandleMetadata = (handle: Handle) => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Thats one weird data structure 😅

Comment on lines -9 to +12
if (!Asset.util.isValidHandle(handle)) throw new InvalidStringError(`Invalid handle ${handle}`);
if (!Asset.util.isValidHandle(handle)) {
logger.warn(`Invalid handle: '${handle}' / '${assetName}'`);
return null;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Just wondering if we wish to do something with these invalid handles. I kinda wish that we had some queue in this project, we could just submit it to it and at some point review whatever invalid handles we find

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that would be nice, but it's a bigger task. This will at least unblock deployments for now.

* Dash: -
* Underscore: _
* Period: .
* Pipe: | (in general it is not valid, but it exists on mainnet)
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's worth wording like 'an exception exists in... '

Copy link
Member Author

@mkazlauskas mkazlauskas Feb 9, 2024

Choose a reason for hiding this comment

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

5e1fcc2 (already squashed)

@@ -23,6 +27,12 @@ const invalidHandles = [
'ada%1_test',
'lace ',
'wallet ',
'comma,',
'comma,@sub',
'sub@comma,',
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

VanessaPC
VanessaPC previously approved these changes Feb 8, 2024
Copy link
Contributor

@VanessaPC VanessaPC left a comment

Choose a reason for hiding this comment

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

Looks good!

@mkazlauskas mkazlauskas force-pushed the fix/handle-projector-crash-on-invalid-name branch 2 times, most recently from 5712653 to 4deea13 Compare February 9, 2024 06:36
iccicci
iccicci previously approved these changes Feb 9, 2024
Copy link
Contributor

@iccicci iccicci left a comment

Choose a reason for hiding this comment

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

Excellent!

VanessaPC
VanessaPC previously approved these changes Feb 9, 2024
Copy link
Contributor

@VanessaPC VanessaPC left a comment

Choose a reason for hiding this comment

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

Great!

- validate length
- do not allow comma as invalid handle
- allow '0|0' as an exception since it exists on mainnet

also add whitespace cases in for handle tests and add doc comment
and update local network handles to be minted as lowercase
@mkazlauskas mkazlauskas dismissed stale reviews from VanessaPC and iccicci via 726f945 February 9, 2024 09:41
@mkazlauskas mkazlauskas force-pushed the fix/handle-projector-crash-on-invalid-name branch from 4deea13 to 726f945 Compare February 9, 2024 09:41
@mkazlauskas mkazlauskas merged commit edf64e5 into master Feb 12, 2024
8 checks passed
@mkazlauskas mkazlauskas deleted the fix/handle-projector-crash-on-invalid-name branch February 12, 2024 11:39
Copy link
Contributor

@gytis-ivaskevicius gytis-ivaskevicius left a comment

Choose a reason for hiding this comment

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

Looks good :))

Comment on lines +1 to 26
/* eslint-disable unicorn/better-regex */
// From ADA Handle Discord: slightly modified to not allow uppercase characters
const REGEX_HANDLE = new RegExp(/^[a-z0-9_.-]{1,15}$/);
// From ADA Handle Discord
const REGEX_SUB_HANDLE = new RegExp(/(?:^[a-z0-9_.-]{1,15}$)|(?:^(?!.{29})[a-z0-9_.-]+@[a-z0-9_.-]{1,15}$)/g);

/**
* From ADA Handle FAQ:
* Alphanumeric: [a-z][0-9]
* Dash: -
* Underscore: _
* Period: .
*
* Since handles are case-insensitive, this function only allows lowercase.
* Max 1-15 characters (or 3-31 characters for a subhandle)
*/
export const isValidHandle = (handle: string) => {
const pattern = /^[\w,.\-]*@{0,1}[\w,.\-]+$/;
return pattern.test(handle);
// 'g' modifier makes it stateful
REGEX_SUB_HANDLE.lastIndex = 0;
return (
REGEX_HANDLE.test(handle) ||
REGEX_SUB_HANDLE.test(handle) ||
// Pipe | in general is not valid, but an exception exists in mainnet
handle === '0|0'
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

I just wanted to mention that there were some cases in my career where I shot myself in the foot with schema validation, as in colleagues push data to database without appropriate checks. To be fair this is a simple schema with one entrypoint

Anyway, even tho it is considered a bad practice I like to enforce it in the database with constraint/check syntax or with triggers

In this case it would be trigger on insert which would set is_valid column to true/false depending on the handle itself

Just a suggestion, and in this case I don't think that its necessary due to simplicity of the schema. I just would like you guys to consider this in the future

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.

4 participants