fix: entity deployment rate-limiter (#1873)

* fix: entity deployment rate-limiter

* fix: tests

* test: fix and add suite for rate-limiting

* chore: apply PR feedback

Author: aleortega

content/src/components.ts

@@ -143,7 +143,8 @@ export async function initComponentsWithEnv(env: Environment): Promise<AppCompon
       entitiesConfigTtl:
         env.getConfig<Map<EntityType, number>>(EnvironmentConfig.DEPLOYMENT_RATE_LIMIT_TTL) ?? new Map(),
       entitiesConfigMax:
-        env.getConfig<Map<EntityType, number>>(EnvironmentConfig.DEPLOYMENT_RATE_LIMIT_MAX) ?? new Map()
+        env.getConfig<Map<EntityType, number>>(EnvironmentConfig.DEPLOYMENT_RATE_LIMIT_MAX) ?? new Map(),
+      entitiesConfigUnchangedTtl: new Map([[EntityType.PROFILE, ms('5m')]]) // ms, converted to seconds internally
     }
   )
 

content/src/ports/deployRateLimiterComponent.ts

@@ -7,13 +7,16 @@ import { AppComponents } from '../types'
 export type IDeployRateLimiterComponent = {
   newDeployment(entityType: EntityType, pointers: string[], localTimestamp: number): void
   isRateLimited(entityType: EntityType, pointers: string[]): boolean
+  newUnchangedDeployment(entityType: EntityType, pointers: string[], localTimestamp: number): void
+  isUnchangedDeploymentRateLimited(entityType: EntityType, pointers: string[]): boolean
 }
 
 export type DeploymentRateLimitConfig = {
   defaultTtl: number
   defaultMax: number
   entitiesConfigTtl: Map<EntityType, number>
   entitiesConfigMax: Map<EntityType, number>
+  entitiesConfigUnchangedTtl: Map<EntityType, number>
 }
 
 export function createDeployRateLimiter(
@@ -27,6 +30,11 @@ export function createDeployRateLimiter(
     rateLimitConfig
   )
 
+  const unchangedDeploymentCacheMap: Map<EntityType, NodeCache> = generateUnchangedDeploymentCacheMap(
+    logs,
+    rateLimitConfig
+  )
+
   function getCacheFromEntityType(entityType: EntityType): { cache: NodeCache; maxSize: number } {
     const cache = deploymentCacheMap.get(entityType)
     if (!cache) {
@@ -35,10 +43,18 @@ export function createDeployRateLimiter(
     return cache
   }
 
+  function getUnchangedCacheFromEntityType(entityType: EntityType): NodeCache {
+    const cache = unchangedDeploymentCacheMap.get(entityType)
+    if (!cache) {
+      throw new Error(`Invalid Entity Type: ${entityType}`)
+    }
+    return cache
+  }
+
   return {
     newDeployment(entityType: EntityType, pointers: string[], localTimestamp: number): void {
       const cacheByEntityType = getCacheFromEntityType(entityType)
-      for (const pointer in pointers) {
+      for (const pointer of pointers) {
         cacheByEntityType.cache.set(pointer, localTimestamp)
       }
     },
@@ -51,10 +67,46 @@ export function createDeployRateLimiter(
         pointers.some((p) => !!cacheByEntityType.cache.get(p)) ||
         cacheByEntityType.cache.stats.keys > cacheByEntityType.maxSize
       )
+    },
+
+    newUnchangedDeployment(entityType: EntityType, pointers: string[], localTimestamp: number): void {
+      const cache = getUnchangedCacheFromEntityType(entityType)
+      for (const pointer of pointers) {
+        cache.set(pointer, localTimestamp)
+      }
+    },
+
+    isUnchangedDeploymentRateLimited(entityType: EntityType, pointers: string[]): boolean {
+      const cache = getUnchangedCacheFromEntityType(entityType)
+      return pointers.some((p) => !!cache.get(p))
     }
   }
 }
 
+function generateUnchangedDeploymentCacheMap(
+  logs: ILoggerComponent.ILogger,
+  rateLimitConfig: DeploymentRateLimitConfig
+): Map<EntityType, NodeCache> {
+  const unchangedCacheMap: Map<EntityType, NodeCache> = new Map()
+
+  for (const entityType of Object.values(EntityType)) {
+    const ttl: number = toSeconds(rateLimitConfig.entitiesConfigUnchangedTtl.get(entityType) ?? 0)
+    unchangedCacheMap.set(entityType, new NodeCache({ stdTTL: ttl, checkperiod: ttl }))
+  }
+
+  const configEntries: string[] = []
+  for (const [entityType, cache] of unchangedCacheMap) {
+    if (cache.options.stdTTL && cache.options.stdTTL > 0) {
+      configEntries.push(`${entityType}: { unchanged_ttl: ${cache.options.stdTTL} }`)
+    }
+  }
+  if (configEntries.length > 0) {
+    logs.info(`Unchanged deployment rate limit configured for:\n${configEntries.join('\n')}`)
+  }
+
+  return unchangedCacheMap
+}
+
 function generateDeploymentCacheMap(
   logs: ILoggerComponent.ILogger,
   rateLimitConfig: DeploymentRateLimitConfig
@@ -97,6 +149,13 @@ function toString(deploymentCacheMap: Map<EntityType, { cache: NodeCache; maxSiz
   return stringifyMap.join('\n')
 }
 
+/**
+ * Convert milliseconds to seconds for NodeCache stdTTL which expects seconds.
+ */
+function toSeconds(milliseconds: number): number {
+  return Math.floor(milliseconds / 1000)
+}
+
 function getCacheConfigPerEntityMap(
   entitiesConfigMax: Map<EntityType, number>,
   entitiesConfigTtl: Map<EntityType, number>
@@ -106,28 +165,28 @@ function getCacheConfigPerEntityMap(
       EntityType.PROFILE,
       {
         max: entitiesConfigMax.get(EntityType.PROFILE) ?? 300,
-        ttl: entitiesConfigTtl.get(EntityType.PROFILE) ?? ms('1m')
+        ttl: toSeconds(entitiesConfigTtl.get(EntityType.PROFILE) ?? ms('15s'))
       }
     ],
     [
       EntityType.SCENE,
       {
         max: entitiesConfigMax.get(EntityType.SCENE) ?? 100000,
-        ttl: entitiesConfigTtl.get(EntityType.SCENE) ?? ms('20s')
+        ttl: toSeconds(entitiesConfigTtl.get(EntityType.SCENE) ?? ms('20s'))
       }
     ],
     [
       EntityType.WEARABLE,
       {
         max: entitiesConfigMax.get(EntityType.WEARABLE) ?? 100000,
-        ttl: entitiesConfigTtl.get(EntityType.WEARABLE) ?? ms('20s')
+        ttl: toSeconds(entitiesConfigTtl.get(EntityType.WEARABLE) ?? ms('20s'))
       }
     ],
     [
       EntityType.STORE,
       {
         max: entitiesConfigMax.get(EntityType.STORE) ?? 300,
-        ttl: entitiesConfigTtl.get(EntityType.STORE) ?? ms('1m')
+        ttl: toSeconds(entitiesConfigTtl.get(EntityType.STORE) ?? ms('1m'))
       }
     ]
   ])

content/src/ports/deployer.ts

@@ -1,6 +1,7 @@
 import { bufferToStream } from '@dcl/catalyst-storage/dist/content-item'
 import { AuthChain, Authenticator } from '@dcl/crypto'
 import { Entity, EntityType, IPFSv2 } from '@dcl/schemas'
+import { isDeepStrictEqual } from 'util'
 import { EnvironmentConfig } from '../Environment'
 import {
   AuditInfo,
@@ -24,6 +25,17 @@ export function isIPFSHash(hash: string): boolean {
   return IPFSv2.validate(hash)
 }
 
+/**
+ * Compare two entities' metadata using deep equality (order-independent).
+ * id, timestamp, version and pointers are top-level Entity fields, not
+ * inside metadata, so they are excluded automatically.
+ * Since ADR-290, profiles no longer carry content files so only metadata
+ * is compared.
+ */
+export function isEntityContentUnchanged(newEntity: Entity, activeEntity: Entity): boolean {
+  return isDeepStrictEqual(newEntity.metadata, activeEntity.metadata)
+}
+
 /**
  * This function will take some deployment files and hash them. They might come already hashed, and if that is the case we will just return them.
  * They could come hashed because the denylist decorator might have already hashed them for its own validations. In order to avoid re-hashing
@@ -93,12 +105,20 @@ export function createDeployer(
     entity: Entity,
     auditInfo: LocalDeploymentAuditInfo,
     hashes: Map<string, Uint8Array>,
-    context: DeploymentContext
+    context: DeploymentContext,
+    isContentUnchanged: boolean
   ): Promise<InvalidResult | { auditInfoComplete: AuditInfo; wasEntityDeployed: boolean }> {
     const deployedEntity = await getEntityById(database, entityId)
     const isEntityAlreadyDeployed = !!deployedEntity
 
-    const validationResult = await validateDeployment(entity, context, isEntityAlreadyDeployed, auditInfo, hashes)
+    const validationResult = await validateDeployment(
+      entity,
+      context,
+      isEntityAlreadyDeployed,
+      auditInfo,
+      hashes,
+      isContentUnchanged
+    )
 
     if (!validationResult.ok) {
       logger.warn(`Validations for deployment failed`, {
@@ -202,14 +222,18 @@ export function createDeployer(
     context: DeploymentContext,
     isEntityDeployedAlready: boolean,
     auditInfo: LocalDeploymentAuditInfo,
-    hashes: Map<string, Uint8Array>
+    hashes: Map<string, Uint8Array>,
+    isContentUnchanged: boolean
   ): Promise<{ ok: boolean; errors?: string[] }> {
     // When deploying a new entity in some context which is not sync, we run some server side checks
     const serverValidationResult = await components.serverValidator.validate(entity, context, {
       areThereNewerEntities: (entity) => areThereNewerEntitiesOnPointers(entity),
       isEntityDeployedAlready: () => isEntityDeployedAlready,
       isNotFailedDeployment: (entity) => components.failedDeployments.findFailedDeployment(entity.id) === undefined,
-      isEntityRateLimited: (entity) => components.deployRateLimiter.isRateLimited(entity.type, entity.pointers),
+      isEntityRateLimited: (entity) =>
+        components.deployRateLimiter.isRateLimited(entity.type, entity.pointers) ||
+        (isContentUnchanged &&
+          components.deployRateLimiter.isUnchangedDeploymentRateLimited(entity.type, entity.pointers)),
       isRequestTtlBackwards: (entity) =>
         components.clock.now() - entity.timestamp >
         components.env.getConfig<number>(EnvironmentConfig.REQUEST_TTL_BACKWARDS)
@@ -292,6 +316,19 @@ export function createDeployer(
 
       const contextToDeploy: DeploymentContext = calculateIfLegacy(entity, auditInfo.authChain, context)
 
+      // Check if the entity content is unchanged from the currently active entity
+      let isContentUnchanged = false
+      if (context === DeploymentContext.LOCAL) {
+        try {
+          const activeEntities = await components.activeEntities.withPointers(components.database, entity.pointers)
+          if (activeEntities.length > 0) {
+            isContentUnchanged = isEntityContentUnchanged(entity, activeEntities[0])
+          }
+        } catch (error) {
+          logger.warn(`Failed to check if entity content is unchanged, assuming changed`, { entityId })
+        }
+      }
+
       try {
         logger.info(`Deploying entity`, {
           entityId,
@@ -304,7 +341,8 @@ export function createDeployer(
           entity,
           auditInfo,
           hashes,
-          contextToDeploy
+          contextToDeploy,
+          isContentUnchanged
         )
 
         if (!storeResult) {
@@ -348,6 +386,14 @@ export function createDeployer(
             entity.pointers,
             storeResult.auditInfoComplete.localTimestamp
           )
+
+          if (isContentUnchanged) {
+            components.deployRateLimiter.newUnchangedDeployment(
+              entity.type,
+              entity.pointers,
+              storeResult.auditInfoComplete.localTimestamp
+            )
+          }
         }
 
         // add the entity to the bloom filter to prevent expensive operations during the sync

content/test/integration/E2ETestEnvironment.ts

@@ -13,6 +13,7 @@ import { DAOComponent } from '../../src/ports/dao-servers-getter'
 import { IDatabaseComponent, createDatabaseComponent } from '../../src/ports/postgres'
 import { AppComponents } from '../../src/types'
 import { MockedDAOClient } from '../helpers/service/synchronization/clients/MockedDAOClient'
+import { createNoOpDeployRateLimiter } from '../mocks/deploy-rate-limiter-mock'
 import { TestProgram } from './TestProgram'
 
 export class E2ETestEnvironment {
@@ -195,6 +196,9 @@ export class ServerBuilder {
         jest.spyOn(components.daoClient, 'getAllServers').mockImplementation(() => this.dao.getAllServers())
       }
 
+      // Override methods on the existing rate limiter object (not replace it)
+      // because the deployer captures its own reference to the original object
+      Object.assign(components.deployRateLimiter, createNoOpDeployRateLimiter())
       servers[i] = new TestProgram(components)
       this.testEnvCalls.registerServer(servers[i])
     }