Skip to content

fix: fix collision handling in comlink username task (backport #3293) #3295

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.

Sign up for GitHub

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

Merged
jusbar23 merged 1 commit into from
Dec 23, 2025
Merged

fix: fix collision handling in comlink username task (backport #3293) #3295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 19 additions & 0 deletions indexer/packages/postgres/src/stores/subaccount-usernames-table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,25 @@ export async function create(
}).returning('*');
}

export async function insertAndReturnCount(
username: string,
subaccountId: string,
options: Options = { txId: undefined },
): Promise<number> {
const queryString: string = `
INSERT INTO subaccount_usernames (username, "subaccountId")
VALUES (?, ?)
ON CONFLICT (username) DO NOTHING
RETURNING username, "subaccountId";
`;

const result: {
rows: { username: string, subaccountId: string }[],
} = await rawQuery(queryString, { ...options, bindings: [username, subaccountId] });

return result.rows.length;
}

export async function findByUsername(
username: string,
options: Options = DEFAULT_POSTGRES_OPTIONS,
Expand Down
160 changes: 143 additions & 17 deletions indexer/services/roundtable/__tests__/tasks/subaccount-username-generator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@ import {
SubaccountUsernamesTable,
SubaccountTable,
QueryableField,

testMocks,
dbHelpers,
SubaccountUsernamesFromDatabase,
SubaccountFromDatabase,
testConstants,
} from '@dydxprotocol-indexer/postgres';
Expand All @@ -21,8 +19,12 @@ describe('subaccount-username-generator', () => {
await testMocks.seedData();
await testMocks.seedAdditionalSubaccounts();
// delete all usernames that were seeded
await SubaccountUsernamesTable.deleteBySubaccountId(testConstants.defaultSubaccountId);
await SubaccountUsernamesTable.deleteBySubaccountId(testConstants.defaultSubaccountId2);
await SubaccountUsernamesTable.deleteBySubaccountId(
testConstants.defaultSubaccountId,
);
await SubaccountUsernamesTable.deleteBySubaccountId(
testConstants.defaultSubaccountId2,
);
});

afterAll(async () => {
Expand All @@ -36,30 +38,154 @@ describe('subaccount-username-generator', () => {
});

it('Successfully creates a username for all subaccount', async () => {
const subaccounts: SubaccountFromDatabase[] = await
SubaccountTable.findAll({
subaccountNumber: 0,
}, [QueryableField.SUBACCOUNT_NUMBER], {});
const subaccounts: SubaccountFromDatabase[] = await SubaccountTable.findAll(
{
subaccountNumber: 0,
},
[QueryableField.SUBACCOUNT_NUMBER],
{},
);

const subaccountsLength: number = subaccounts.length;
const subaccountsWithUsernames: SubaccountUsernamesFromDatabase[] = await
SubaccountUsernamesTable.findAll(
{}, [], {});
expect(subaccountsWithUsernames.length).toEqual(0);
const before = await SubaccountUsernamesTable.findAll(
{},
[],
{ readReplica: true },
);
expect(before.length).toEqual(0);

await subaccountUsernameGenerator();
const subaccountsWithUsernamesAfter: SubaccountUsernamesFromDatabase[] = await
SubaccountUsernamesTable.findAll(
{}, [], {});
const after = await SubaccountUsernamesTable.findAll(
{},
[],
{ readReplica: true },
);

const expectedUsernames = [
'BubblyEarH5Y', // dydx1n88uc38xhjgxzw9nwre4ep2c8ga4fjxc575lnf
'GreenSnowWTT', // dydx1n88uc38xhjgxzw9nwre4ep2c8ga4fjxc565lnf
'LunarMatFK5', // dydx199tqg4wdlnu4qjlxchpd7seg454937hjrknju4
];
expect(subaccountsWithUsernamesAfter.length).toEqual(subaccountsLength);
expect(after.length).toEqual(subaccountsLength);
for (let i = 0; i < expectedUsernames.length; i++) {
expect(subaccountsWithUsernamesAfter[i].username).toEqual(expectedUsernames[i]);
expect(after[i].username).toEqual(expectedUsernames[i]);
}
});

it('Falls back to a second username when there is a conflict on the first attempt', async () => {
const subaccounts: SubaccountFromDatabase[] = await SubaccountTable.findAll(
{
subaccountNumber: 0,
},
[QueryableField.SUBACCOUNT_NUMBER],
{},
);
const targetSubaccount = subaccounts[0];
const otherSubaccount = subaccounts[1];

const { generateUsernameForSubaccount } = require('../../src/helpers/usernames-helper');

const usernameAttempt0 = generateUsernameForSubaccount(
targetSubaccount.address,
0,
0,
);
await SubaccountUsernamesTable.create({
username: usernameAttempt0,
subaccountId: otherSubaccount.id,
});

const afterPreInsert = await SubaccountUsernamesTable.findAll(
{ subaccountId: [targetSubaccount.id] }, [QueryableField.SUBACCOUNT_ID], {},
);
expect(afterPreInsert.length).toBe(0);

await subaccountUsernameGenerator();

const created = await SubaccountUsernamesTable.findAll(
{ subaccountId: [targetSubaccount.id] }, [QueryableField.SUBACCOUNT_ID], {},
);
expect(created.length).toBe(1);

const fallbackUsername = generateUsernameForSubaccount(
targetSubaccount.address,
0,
1,
);
expect(created[0].username).toEqual(fallbackUsername);

const conflict = await SubaccountUsernamesTable.findAll(
{ username: [usernameAttempt0] }, [QueryableField.USERNAME], {},
);
expect(conflict.length).toBe(1);
expect(conflict[0].subaccountId).not.toEqual(targetSubaccount.id);
});

it('Handles batch where one username succeeds and the other needs a fallback', async () => {
const subaccounts: SubaccountFromDatabase[] = await SubaccountTable.findAll(
{
subaccountNumber: 0,
},
[QueryableField.SUBACCOUNT_NUMBER],
{},
);
expect(subaccounts.length).toBeGreaterThanOrEqual(2);

const sub0 = subaccounts[0];
const sub1 = subaccounts[1];

const { generateUsernameForSubaccount } = require('../../src/helpers/usernames-helper');

const sub0Attempt0 = generateUsernameForSubaccount(sub0.address, 0, 0);
await SubaccountUsernamesTable.create({
username: sub0Attempt0,
subaccountId: sub1.id,
});

// pre-run checks
const preUsernames = await SubaccountUsernamesTable.findAll(
{},
[],
{ readReplica: true },
);
expect(
preUsernames.find((u: any) => u.username === sub0Attempt0),
).toBeDefined();
expect(preUsernames.filter((u: any) => u.subaccountId === sub0.id).length).toBe(0);
expect(preUsernames.filter(
(u: any) => u.subaccountId === sub1.id && u.username !== sub0Attempt0,
).length).toBe(0);

// run generator
await subaccountUsernameGenerator();

// fetch results
const after = await SubaccountUsernamesTable.findAll(
{},
[],
{ readReplica: true },
);

// sub0 should have fallback username
const sub0UsernameRow = after.find((u: any) => u.subaccountId === sub0.id);
const sub0ExpectedFallback = generateUsernameForSubaccount(sub0.address, 0, 1);
expect(sub0UsernameRow).toBeDefined();
if (sub0UsernameRow) {
expect(sub0UsernameRow.username).toEqual(sub0ExpectedFallback);
}
const sub1UsernameRow = after.find(
(u: any) => u.subaccountId === sub1.id && u.username !== sub0Attempt0);
if (sub1UsernameRow) {
expect(sub1UsernameRow).toBeDefined();
}

// There should not be two usernames with the same value
const usernameCounts = after.reduce((acc: Record<string, number>, u: any) => {
acc[u.username] = (acc[u.username] || 0) + 1;
return acc;
}, {});
for (const count of Object.values(usernameCounts)) {
expect(count).toBe(1);
}
});
});
54 changes: 25 additions & 29 deletions indexer/services/roundtable/src/tasks/subaccount-username-generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { logger, stats } from '@dydxprotocol-indexer/base';
import {
SubaccountUsernamesTable,
SubaccountsWithoutUsernamesResult,
Transaction,
} from '@dydxprotocol-indexer/postgres';
import _ from 'lodash';
Expand All @@ -12,9 +11,7 @@ import { generateUsernameForSubaccount } from '../helpers/usernames-helper';
export default async function runTask(): Promise<void> {
const taskStart: number = Date.now();

const subaccountZerosWithoutUsername:
SubaccountsWithoutUsernamesResult[] = await
SubaccountUsernamesTable.getSubaccountZerosWithoutUsernames(
const targetAccounts = await SubaccountUsernamesTable.getSubaccountZerosWithoutUsernames(
config.SUBACCOUNT_USERNAME_BATCH_SIZE,
);
stats.timing(
Expand All @@ -26,7 +23,7 @@ export default async function runTask(): Promise<void> {
const txnStart: number = Date.now();
try {
let successCount: number = 0;
for (const subaccount of subaccountZerosWithoutUsername) {
for (const subaccount of targetAccounts) {
for (let i = 0; i < config.ATTEMPT_PER_SUBACCOUNT; i++) {
const username: string = generateUsernameForSubaccount(
subaccount.address,
Expand All @@ -38,43 +35,42 @@ export default async function runTask(): Promise<void> {
i,
);
try {
await SubaccountUsernamesTable.create({
const count: number = await SubaccountUsernamesTable.insertAndReturnCount(
username,
subaccountId: subaccount.subaccountId,
}, { txId });
// If success, break from loop and move to next subaccount.
successCount += 1;
break;
} catch (e) {
// There are roughly ~225 million possible usernames
// so the chance of collision is very low.
if (e instanceof Error && e.name === 'UniqueViolationError') {
stats.increment(
`${config.SERVICE_NAME}.subaccount-username-generator.collision`, 1);
logger.info({
at: 'subaccount-username-generator#runTask',
message: 'username collision',
address: subaccount.address,
subaccountId: subaccount.subaccountId,
username,
error: e,
});
subaccount.subaccountId,
{ txId },
);
if (count > 0) {
successCount += 1;
break;
} else {
// if count is 0, log error and continue to next iteration
// which will bump the nonce and try again with a new username
logger.error({
at: 'subaccount-username-generator#runTask',
message: 'Failed to insert username for subaccount',
address: subaccount.address,
subaccountId: subaccount.subaccountId,
username,
error: e,
error: new Error('Username already exists'),
});
}
} catch (e) {
logger.error({
at: 'subaccount-username-generator#runTask',
message: 'Failed to insert username for subaccount',
address: subaccount.address,
subaccountId: subaccount.subaccountId,
username,
error: e,
});
throw e;
}
}
}
await Transaction.commit(txId);
const subaccountAddresses = _.map(
subaccountZerosWithoutUsername,
targetAccounts,
(subaccount) => subaccount.address,
);
stats.timing(
Expand All @@ -84,7 +80,7 @@ export default async function runTask(): Promise<void> {
logger.info({
at: 'subaccount-username-generator#runTask',
message: 'Generated usernames',
batchSize: subaccountZerosWithoutUsername.length,
batchSize: targetAccounts.length,
successCount,
addressSample: subaccountAddresses.slice(0, 10),
duration: Date.now() - taskStart,
Expand All @@ -93,7 +89,7 @@ export default async function runTask(): Promise<void> {
await Transaction.rollback(txId);
logger.error({
at: 'subaccount-username-generator#runTask',
message: 'Error when updating totalVolume in wallets table',
message: 'Error when generating usernames for subaccounts',
error,
});
}
Expand Down
Loading