Skip to content

Commit

Permalink
fix: Scene entity metadata owner can be empty (#116)
Browse files Browse the repository at this point in the history
* fix: Open for Business avoid increasing steps when already completed

* fix: Scene Owner can be empty and it should not

* fix: Event Parser includes the auth chain for catalyst deployment events

* fix: Take the owner address from the auth chain on LAND Architect handler

* chore: Bump schemas

* fix: Take the owner address from the auth chain on Open for Business handler

* feat: Validate if the owner address from the auth chain is valid

* fix: Other tests mocking the same event without the authchain
  • Loading branch information
kevinszuchet authored Oct 15, 2024
1 parent e6dc6d9 commit 6fac521
Show file tree
Hide file tree
Showing 13 changed files with 297 additions and 175 deletions.
2 changes: 1 addition & 1 deletion api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
},
"dependencies": {
"@dcl/platform-server-commons": "^0.0.4",
"@dcl/schemas": "^14.1.0",
"@dcl/schemas": "^15.0.0",
"@well-known-components/env-config-provider": "^1.2.0",
"@well-known-components/fetch-component": "^2.0.2",
"@well-known-components/http-server": "^2.1.0",
Expand Down
2 changes: 1 addition & 1 deletion common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"lint:fix": "eslint '**/*.{js,ts}' --fix"
},
"dependencies": {
"@dcl/schemas": "^14.1.0",
"@dcl/schemas": "^15.0.0",
"@well-known-components/interfaces": "^1.4.3"
},
"devDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion processor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"dependencies": {
"@aws-sdk/client-sns": "^3.600.0",
"@aws-sdk/client-sqs": "^3.600.0",
"@dcl/schemas": "^14.1.0",
"@dcl/schemas": "^15.0.0",
"@dcl/urn-resolver": "^3.6.0",
"@well-known-components/env-config-provider": "^1.2.0",
"@well-known-components/fetch-component": "^2.0.2",
Expand Down
3 changes: 2 additions & 1 deletion processor/src/adapters/event-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ export async function createEventParser({
subType: event.entity.entityType,
key: event.entity.entityId,
timestamp: fetchedEntity.timestamp,
entity: fetchedEntity
entity: fetchedEntity,
authChain: event.authChain
} as CatalystDeploymentEvent
}

Expand Down
10 changes: 8 additions & 2 deletions processor/src/logic/badges/land-architect.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CatalystDeploymentEvent, EthAddress, Events } from '@dcl/schemas'
import { AppComponents, BadgeProcessorResult, IObserver } from '../../types'
import { Authenticator } from '@dcl/crypto'
import { Badge, BadgeId, UserBadge } from '@badges/common'
import { AppComponents, BadgeProcessorResult, IObserver } from '../../types'

export function createLandArchitectObserver({
db,
Expand All @@ -12,7 +13,7 @@ export function createLandArchitectObserver({
const badge: Badge = badgeStorage.getBadge(badgeId)

function getUserAddress(event: CatalystDeploymentEvent): EthAddress {
return event.entity.metadata.owner
return Authenticator.ownerAddress(event.authChain)
}

async function handle(
Expand All @@ -21,6 +22,11 @@ export function createLandArchitectObserver({
): Promise<BadgeProcessorResult | undefined> {
const userAddress = getUserAddress(event)

if (!EthAddress.validate(userAddress)) {
logger.error('Invalid user address', { userAddress, badgeId, eventType: event.type })
return undefined
}

userProgress ||= initProgressFor(userAddress)

if (userProgress.completed_at) {
Expand Down
26 changes: 22 additions & 4 deletions processor/src/logic/badges/open-for-business.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CatalystDeploymentEvent, CollectionCreatedEvent, EthAddress, Events } from '@dcl/schemas'
import { AppComponents, BadgeProcessorResult, IObserver } from '../../types'
import { Authenticator } from '@dcl/crypto'
import { Badge, BadgeId, UserBadge } from '@badges/common'
import { AppComponents, BadgeProcessorResult, IObserver } from '../../types'

export function createOpenForBusinessObserver({
db,
Expand All @@ -13,11 +14,12 @@ export function createOpenForBusinessObserver({

const functionsPerEvent = {
[Events.Type.CATALYST_DEPLOYMENT]: (event: any) => ({
getUserAddress: () => event.entity.metadata.owner,
getUserAddress: () => Authenticator.ownerAddress(event.authChain),
updateUserProgress: (userProgress: UserBadge) => ({
...userProgress,
progress: { ...userProgress.progress, steps: (userProgress.progress.steps || 0) + 1, store_completed: true }
})
}),
stepAlreadyCompleted: (userProgress: UserBadge) => userProgress.progress.store_completed
}),
[Events.Type.BLOCKCHAIN]: (event: any) => ({
getUserAddress: () => event.metadata.creator,
Expand All @@ -28,7 +30,8 @@ export function createOpenForBusinessObserver({
steps: (userProgress.progress.steps || 0) + 1,
collection_submitted: true
}
})
}),
stepAlreadyCompleted: (userProgress: UserBadge) => userProgress.progress.collection_submitted
})
}

Expand All @@ -44,6 +47,11 @@ export function createOpenForBusinessObserver({
const functions = functionsPerEvent[event.type](event)
const userAddress: EthAddress = getUserAddress(event)

if (!EthAddress.validate(userAddress)) {
logger.error('Invalid user address', { userAddress, badgeId, eventType: event.type })
return undefined
}

userProgress ||= initProgressFor(userAddress)

if (userProgress.completed_at) {
Expand All @@ -55,6 +63,16 @@ export function createOpenForBusinessObserver({
return undefined
}

if (functions.stepAlreadyCompleted(userProgress)) {
logger.info('User already completed the step associated to this event', {
userAddress: userAddress!,
badgeId: badgeId,
eventType: event.type
})

return undefined
}

const updatedUserProgress = functions.updateUserProgress(userProgress)

if (updatedUserProgress.progress.store_completed && updatedUserProgress.progress.collection_submitted) {
Expand Down
10 changes: 10 additions & 0 deletions processor/src/utils/authenticator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { AuthChain, EthAddress } from '@dcl/schemas'
import { Authenticator } from '@dcl/crypto'

export function getOwnerAddressFromAuthChain(authChain: AuthChain): string {
return Authenticator.ownerAddress(authChain)
}

export function isValidOwnerAddress(ownerAddress: EthAddress): boolean {
return !!ownerAddress && EthAddress.validate(ownerAddress)
}
27 changes: 19 additions & 8 deletions processor/test/unit/adapters/event-parser.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { createLogComponent } from '@well-known-components/logger'
import { AppComponents, ParsingEventError } from '../../../src/types'
import { createEventParser, SubType } from '../../../src/adapters/event-parser'
import { CatalystDeploymentEvent, Events } from '@dcl/schemas'
import * as CatalystClient from 'dcl-catalyst-client'
import { AuthChain, AuthLinkType, Events } from '@dcl/schemas'

jest.mock('dcl-catalyst-client')

Expand Down Expand Up @@ -60,6 +59,13 @@ describe('Event Parser', () => {
}
}

const mockAuthChain: AuthChain = [
{
payload: 'auth-chain-payload',
type: AuthLinkType.SIGNER
}
]

it('and the entity type is not included in the subtypes should return undefined', async () => {
const { config, logs, badgeContext } = await getMockedComponents()
const parser = await createEventParser({ config, logs, badgeContext })
Expand All @@ -75,7 +81,8 @@ describe('Event Parser', () => {
const { config, logs, badgeContext } = await getMockedComponents()
const parser = await createEventParser({ config, logs, badgeContext })
const event = {
entity: { entityId: 'some-id', pointers: ['0xTest'], entityType: Events.SubType.CatalystDeployment.PROFILE }
entity: { entityId: 'some-id', pointers: ['0xTest'], entityType: Events.SubType.CatalystDeployment.PROFILE },
authChain: mockAuthChain
}

badgeContext.getEntitiesByPointers = jest.fn().mockResolvedValue([mockEntity])
Expand All @@ -92,7 +99,8 @@ describe('Event Parser', () => {
subType: Events.SubType.CatalystDeployment.PROFILE,
key: event.entity.entityId,
timestamp: mockEntity.timestamp,
entity: mockEntity
entity: mockEntity,
authChain: mockAuthChain
})
})

Expand All @@ -103,6 +111,7 @@ describe('Event Parser', () => {
const parser = await createEventParser({ config, logs, badgeContext })
const event = {
entity: { entityId: 'some-id', entityType: subType },
authChain: mockAuthChain,
contentServerUrls: ['http://some-url']
}

Expand All @@ -115,8 +124,9 @@ describe('Event Parser', () => {
subType,
key: event.entity.entityId,
timestamp: mockEntity.timestamp,
entity: mockEntity
} as CatalystDeploymentEvent)
entity: mockEntity,
authChain: mockAuthChain
})
}
)

Expand All @@ -127,10 +137,11 @@ describe('Event Parser', () => {
contentServerUrls: ['http://some-url']
}

badgeContext.getEntitiesByPointers = jest.fn().mockRejectedValue(new Error('Error fetching entity from content server'))
badgeContext.getEntitiesByPointers = jest
.fn()
.mockRejectedValue(new Error('Error fetching entity from content server'))
const parser = await createEventParser({ config, logs, badgeContext })


await expect(parser.parse(event)).rejects.toThrow(ParsingEventError)
})
})
Expand Down
26 changes: 22 additions & 4 deletions processor/test/unit/logic/badges/epic-ensemble.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { BadgeId, createBadgeStorage, UserBadge } from '@badges/common'
import { AppComponents } from '../../../../src/types'
import { createDbMock } from '../../../mocks/db-mock'
import { CatalystDeploymentEvent, EntityType, Events } from '@dcl/schemas'
import { AuthLinkType, CatalystDeploymentEvent, EntityType, Events } from '@dcl/schemas'
import { createEpicEnsembleObserver } from '../../../../src/logic/badges/epic-ensemble'
import { createLogComponent } from '@well-known-components/logger'

Expand Down Expand Up @@ -66,7 +66,13 @@ describe('Epic Ensemble badge handler should', () => {
}
]
}
}
},
authChain: [
{
payload: testAddress,
type: AuthLinkType.SIGNER
}
]
}

badgeContext.getWearablesWithRarity = jest
Expand Down Expand Up @@ -137,7 +143,13 @@ describe('Epic Ensemble badge handler should', () => {
}
]
}
}
},
authChain: [
{
payload: testAddress,
type: AuthLinkType.SIGNER
}
]
}

badgeContext.getWearablesWithRarity = jest
Expand Down Expand Up @@ -202,7 +214,13 @@ describe('Epic Ensemble badge handler should', () => {
}
]
}
}
},
authChain: [
{
payload: testAddress,
type: AuthLinkType.SIGNER
}
]
}

const handler = createEpicEnsembleObserver({ db, logs, badgeContext, badgeStorage })
Expand Down
39 changes: 31 additions & 8 deletions processor/test/unit/logic/badges/land-architect.spec.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { createLogComponent } from '@well-known-components/logger'
import { AppComponents } from '../../../../src/types'
import { createDbMock } from '../../../mocks/db-mock'
import { CatalystDeploymentEvent, Events } from '@dcl/schemas'
import { AuthLinkType, CatalystDeploymentEvent, Entity, Events } from '@dcl/schemas'
import { Badge, BadgeId, createBadgeStorage } from '@badges/common'
import { createLandArchitectObserver } from '../../../../src/logic/badges/land-architect'

describe('LAND Architect badge handler should', () => {
const testAddress = '0xTest'
const testAddress = '0x1234567890abcdef1234567890abcdef12345678'

it('grant badge when a user deploy a scene for the first time', async () => {
const { db, logs, badgeStorage } = await getMockedComponents()
Expand Down Expand Up @@ -48,10 +47,31 @@ describe('LAND Architect badge handler should', () => {
expect(result).toBe(undefined)
})

it('do nothing when the auth chain has an invalid owner address', async () => {
const { db, logs, badgeStorage } = await getMockedComponents()

const event = createSceneDeployedEvent()
event.authChain[0].payload = '0xInvalid'

const handler = createLandArchitectObserver({ db, logs, badgeStorage })

const result = await handler.handle(event)

expect(db.saveUserProgress).not.toHaveBeenCalled()
expect(result).toBe(undefined)
})

async function getMockedComponents(): Promise<Pick<AppComponents, 'db' | 'logs' | 'badgeStorage'>> {
return {
db: createDbMock(),
logs: await createLogComponent({ config: { requireString: jest.fn(), getString: jest.fn() } as any }),
logs: {
getLogger: jest.fn().mockReturnValue({
info: jest.fn(),
debug: jest.fn(),
error: jest.fn(),
warn: jest.fn()
})
},
badgeStorage: await createBadgeStorage({
config: { requireString: jest.fn().mockResolvedValue('https://any-url.tld') } as any
})
Expand All @@ -65,11 +85,14 @@ describe('LAND Architect badge handler should', () => {
key: 'aKey',
timestamp: 1708380838534,
entity: {
pointers: ['-105,65'],
metadata: {
owner: testAddress
pointers: ['-105,65']
} as Entity,
authChain: [
{
payload: testAddress,
type: AuthLinkType.SIGNER
}
} as any
]
}
}

Expand Down
Loading

0 comments on commit 6fac521

Please sign in to comment.