Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
-- Make (address, chainId) the real uniqueness key for accounts, and propagate
-- chainId onto transactions so the foreign key targets the composite key.
--
-- Root cause: contract addresses derive from CREATE = keccak256(rlp(sender, nonce)),
-- which does NOT include chainId. The same relayer wallet can therefore produce
-- identical addresses on Horizen and Base, which violated `accounts.address @unique`
-- and blocked multi-chain batch deployments.
--
-- Migration order matters: backfill transactions.chain_id BEFORE swapping
-- constraints, otherwise the new composite FK has nothing to point at.
--
-- Object naming convention follows the init migration: `@@unique` and `@unique`
-- are implemented as UNIQUE INDEXes (so DROP INDEX / CREATE UNIQUE INDEX),
-- while explicit @relation foreign keys are real constraints
-- (DROP CONSTRAINT / ADD CONSTRAINT FOREIGN KEY).

-- 0. Pre-check: refuse to migrate if any transaction references a missing account.
-- A NULL chain_id after backfill would block the NOT NULL step downstream.
DO $$
DECLARE orphan_count INTEGER;
BEGIN
SELECT COUNT(*) INTO orphan_count
FROM transactions t
LEFT JOIN accounts a ON a.address = t.account_address
WHERE a.address IS NULL;

IF orphan_count > 0 THEN
RAISE EXCEPTION 'Found % orphan transactions with no matching account. Resolve manually before re-running.', orphan_count;
END IF;
END $$;

-- 1. Add the new column as NULLABLE so the table accepts the change without a default.
ALTER TABLE "transactions" ADD COLUMN "chain_id" INTEGER;

-- 2. Backfill from the parent account row.
UPDATE "transactions" t
SET "chain_id" = a."chain_id"
FROM "accounts" a
WHERE a."address" = t."account_address";

-- 3. Promote to NOT NULL once every row has a value.
ALTER TABLE "transactions" ALTER COLUMN "chain_id" SET NOT NULL;

-- 4. Drop the old indexes / FK we're replacing.
-- UNIQUE INDEX → DROP INDEX, FK constraint → DROP CONSTRAINT.
ALTER TABLE "transactions" DROP CONSTRAINT "transactions_account_address_fkey";
DROP INDEX "transactions_account_address_nonce_key";
DROP INDEX "accounts_address_key";

-- 5. Replace with composite (address, chainId) uniqueness on accounts.
CREATE UNIQUE INDEX "accounts_address_chain_id_key"
ON "accounts" ("address", "chain_id");

-- 6. Replace transaction-side composite unique + FK so the relation targets (address, chainId).
CREATE UNIQUE INDEX "transactions_account_address_chain_id_nonce_key"
ON "transactions" ("account_address", "chain_id", "nonce");

ALTER TABLE "transactions"
ADD CONSTRAINT "transactions_account_address_chain_id_fkey"
FOREIGN KEY ("account_address", "chain_id")
REFERENCES "accounts" ("address", "chain_id")
ON DELETE RESTRICT ON UPDATE CASCADE;

-- 7. Swap the supporting helper index for the composite key.
DROP INDEX IF EXISTS "transactions_account_address_idx";
CREATE INDEX "transactions_account_address_chain_id_idx"
ON "transactions" ("account_address", "chain_id");

-- 8. Same fix for reserved_nonces. Without chainId, two chains sharing an
-- address would also share a nonce-reservation row and could collide.
ALTER TABLE "reserved_nonces" ADD COLUMN "chain_id" INTEGER;

UPDATE "reserved_nonces" r
SET "chain_id" = a."chain_id"
FROM "accounts" a
WHERE a."address" = r."account_address";

-- Any reservation pointing at an address with no matching account is stale —
-- safe to drop, the system will recreate as needed.
DELETE FROM "reserved_nonces" WHERE "chain_id" IS NULL;

ALTER TABLE "reserved_nonces" ALTER COLUMN "chain_id" SET NOT NULL;

DROP INDEX "reserved_nonces_account_address_nonce_key";

CREATE UNIQUE INDEX "reserved_nonces_account_address_chain_id_nonce_key"
ON "reserved_nonces" ("account_address", "chain_id", "nonce");
17 changes: 12 additions & 5 deletions packages/backend/prisma/schema.prisma
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ model User {

model Account {
id String @id @default(cuid())
address String @unique
address String
name String
threshold Int
chainId Int @default(26514) @map("chain_id")
Expand All @@ -37,6 +37,11 @@ model Account {
contacts Contact[]
transactions Transaction[]

// Contract addresses derive from (relayer, nonce) without including chainId,
// so the same wallet can produce identical addresses on different chains
// (Horizen vs Base) when nonces align. The pair (address, chainId) is the
// real uniqueness key for a deployed multisig.
@@unique([address, chainId])
@@map("accounts")
}

Expand Down Expand Up @@ -64,6 +69,7 @@ model Transaction {
value String?
tokenAddress String? @map("token_address")
accountAddress String @map("account_address")
chainId Int @map("chain_id")
contactId String? @map("contact_id")
signerData String? @map("signer_data")
newThreshold Int? @map("new_threshold")
Expand All @@ -74,25 +80,26 @@ model Transaction {
executedAt DateTime? @map("executed_at")
createdAt DateTime @default(now()) @map("created_at")
updatedAt DateTime @updatedAt @map("updated_at")
account Account @relation(fields: [accountAddress], references: [address])
account Account @relation(fields: [accountAddress, chainId], references: [address, chainId])
contact Contact? @relation(fields: [contactId], references: [id])
votes Vote[]

@@unique([accountAddress, nonce])
@@unique([accountAddress, chainId, nonce])
@@index([status])
@@index([accountAddress])
@@index([accountAddress, chainId])
@@index([contactId])
@@map("transactions")
}

model ReservedNonce {
id String @id @default(cuid())
accountAddress String @map("account_address")
chainId Int @map("chain_id")
nonce Int
expiresAt DateTime @map("expires_at")
createdAt DateTime @default(now()) @map("created_at")

@@unique([accountAddress, nonce])
@@unique([accountAddress, chainId, nonce])
@@map("reserved_nonces")
}

Expand Down
26 changes: 23 additions & 3 deletions packages/backend/src/account/account.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ import {
ApiBearerAuth,
} from '@nestjs/swagger';
import {
BadRequestException,
Controller,
Get,
Post,
Body,
Param,
Patch,
Query,
UseGuards,
} from '@nestjs/common';
import {
Expand Down Expand Up @@ -128,8 +130,12 @@ export class AccountController {
description: 'Forbidden - Not an account signer',
})
@ApiResponse({ status: 404, description: 'Account not found' })
async findByAddress(@Param('address') address: string) {
return this.accountService.findByAddress(address);
async findByAddress(
@Param('address') address: string,
@Query('chainId') chainIdRaw: string,
) {
const chainId = parseChainIdQuery(chainIdRaw);
return this.accountService.findByAddress(address, chainId);
}

/**
Expand Down Expand Up @@ -170,8 +176,22 @@ export class AccountController {
@ApiResponse({ status: 404, description: 'Account not found' })
async update(
@Param('address') address: string,
@Query('chainId') chainIdRaw: string,
@Body() dto: UpdateAccountDto,
) {
return this.accountService.update(address, dto);
const chainId = parseChainIdQuery(chainIdRaw);
return this.accountService.update(address, chainId, dto);
}
}

// (address, chainId) is the real account key; refuse requests that omit it
// rather than silently picking the first match.
function parseChainIdQuery(value: string | undefined): number {
const chainId = Number.parseInt(value ?? '', 10);
if (!Number.isFinite(chainId) || chainId <= 0) {
throw new BadRequestException(
'chainId query parameter is required (e.g. ?chainId=8453)',
);
}
return chainId;
}
22 changes: 14 additions & 8 deletions packages/backend/src/account/account.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export class AccountService {
);
}

return this.findByAddress(address);
return this.findByAddress(address, dto.chainId);
}

async createBatch(
Expand Down Expand Up @@ -309,11 +309,15 @@ export class AccountService {
}

/**
* Get multisig account by address with signers
* Get multisig account by (address, chainId) — the composite uniqueness key.
* The same address can exist on multiple chains (Horizen vs Base) when
* relayer nonces align, so chainId is mandatory to disambiguate.
*/
async findByAddress(address: string) {
async findByAddress(address: string, chainId: number) {
const account = await this.prisma.account.findUnique({
where: { address },
where: {
address_chainId: { address, chainId },
},
include: {
signers: {
include: {
Expand Down Expand Up @@ -383,22 +387,24 @@ export class AccountService {
/**
* Update multisig account by address
*/
async update(address: string, dto: UpdateAccountDto) {
async update(address: string, chainId: number, dto: UpdateAccountDto) {
const account = await this.prisma.account.findUnique({
where: { address },
where: {
address_chainId: { address, chainId },
},
});

if (!account) {
throw new NotFoundException('Account not found');
}

await this.prisma.account.update({
where: { address },
where: { id: account.id },
data: {
name: dto.name,
},
});

return this.findByAddress(address);
return this.findByAddress(address, chainId);
}
}
19 changes: 16 additions & 3 deletions packages/backend/src/auth/guards/account-member.guard.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import {
Injectable,
BadRequestException,
CanActivate,
ExecutionContext,
ForbiddenException,
Injectable,
} from '@nestjs/common';
import { PrismaService } from '@/database/prisma.service';
import { NOT_MEMBER_OF_ACCOUNT } from '@/common/constants';
Expand All @@ -15,6 +16,7 @@ export class AccountMemberGuard implements CanActivate {
const request = context.switchToHttp().getRequest();
const userCommitment = request.user?.commitment;
const accountAddress = request.params?.address;
const chainIdRaw = request.query?.chainId;

if (!userCommitment) {
throw new ForbiddenException('User not authenticated');
Expand All @@ -24,10 +26,21 @@ export class AccountMemberGuard implements CanActivate {
throw new ForbiddenException('Account address not provided');
}

// Check if user is a member of the wallet
// chainId is mandatory: the same address can exist on multiple chains
// (same relayer + same nonce on Horizen vs Base). Without scoping the
// lookup by chainId, a signer of one account could trick the guard into
// letting them act on a different account that happens to share its
// address.
const chainId = Number.parseInt(String(chainIdRaw ?? ''), 10);
if (!Number.isFinite(chainId) || chainId <= 0) {
throw new BadRequestException(
'chainId query parameter is required (e.g. ?chainId=8453)',
);
}

const membership = await this.prisma.accountSigner.findFirst({
where: {
account: { address: accountAddress },
account: { address: accountAddress, chainId },
user: { commitment: userCommitment },
},
});
Expand Down
14 changes: 9 additions & 5 deletions packages/backend/src/auth/guards/transaction-access.guard.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {
Injectable,
CanActivate,
ExecutionContext,
ForbiddenException,
Injectable,
NotFoundException,
} from '@nestjs/common';
import { PrismaService } from '@/database/prisma.service';
Expand All @@ -25,20 +25,24 @@ export class TransactionAccessGuard implements CanActivate {
throw new ForbiddenException('Transaction ID not provided');
}

// Find transaction and its wallet
const transaction = await this.prisma.transaction.findUnique({
where: { txId },
include: { account: true },
select: { accountAddress: true, chainId: true },
});

if (!transaction) {
throw new NotFoundException('Transaction not found');
}

// Check if user is a member of the wallet
// Scope membership by both address AND chainId — the same address can
// live on multiple chains, and only the signer of the exact multisig
// (this transaction's chain) should be allowed access.
const membership = await this.prisma.accountSigner.findFirst({
where: {
account: { address: transaction.account.address },
account: {
address: transaction.accountAddress,
chainId: transaction.chainId,
},
user: { commitment: userCommitment },
},
});
Expand Down
17 changes: 12 additions & 5 deletions packages/backend/src/common/utils/membership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,26 @@ import { NOT_MEMBER_OF_ACCOUNT } from '@/common/constants';

/**
* Check if user is a signer of the account. Throws ForbiddenException if not.
* @param prisma - PrismaService instance
* @param accountLookup - Either { accountId } or { accountAddress } to identify the account
* @param userCommitment - User's commitment string
*
* Account lookup MUST be precise: either `accountId` (a CUID) or the composite
* `{ accountAddress, chainId }`. Using address alone is unsafe — the same
* address can exist on multiple chains, so an address-only check could match
* a signer of a different multisig that happens to share the address.
*/
export async function checkAccountMembership(
prisma: PrismaService,
accountLookup: { accountId: string } | { accountAddress: string },
accountLookup:
| { accountId: string }
| { accountAddress: string; chainId: number },
userCommitment: string,
): Promise<void> {
const accountWhere =
'accountId' in accountLookup
? { id: accountLookup.accountId }
: { address: accountLookup.accountAddress };
: {
address: accountLookup.accountAddress,
chainId: accountLookup.chainId,
};

const membership = await prisma.accountSigner.findFirst({
where: {
Expand Down
6 changes: 4 additions & 2 deletions packages/backend/src/quest/quest.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ export class QuestService {
return 0;
}

// Get account and creator
const account = await this.prisma.account.findUnique({
// Get account and creator. Address is no longer globally unique
// (per-chain), so we use findFirst — quest module is currently disabled
// anyway and ambiguity is acceptable for points accounting.
const account = await this.prisma.account.findFirst({
where: { address: accountAddress },
include: {
signers: {
Expand Down
Loading
Loading