Skip to content

Commit 2ba688b

Browse files
committed
fix(address-comments): derive the Atlas event_id and bound the SQS publish
- src/models/atlas/atlas-event-id.ts — derive event_id as a UUIDv5 of swap_id, event_type and the rsk tx hash, so the MessageDeduplicationId has something to deduplicate on (per @Dominikkq) - src/services/atlas/{pegin,pegout}-atlas-event.builder.ts — use it in place of randomUUID - src/services/atlas/sqs-atlas-event-publisher.ts — bound the publish with connection/request timeouts and maxAttempts, so a hung queue cannot strand the rest of a block (per @Dominikkq); requestTimeout needs throwOnRequestTimeout to abort rather than only warn - ENV_VARIABLES.md — document the three new variables and both fixes
1 parent dd2bf63 commit 2ba688b

12 files changed

Lines changed: 484 additions & 17 deletions

‎.env.test‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ RSK_PEGOUT_MINIMUM_CONFIRMATIONS=10
55
FAST_MINING_BLOCK=1
66
AVERAGE_MINING_BLOCK=6
77
LOW_MINING_BLOCK=12
8-
BLOCKBOOK_URL='https://blockbook-01.testnet.2wp.iovlabs.net:19130/'
8+
BLOCKBOOK_URL='https://blockbook.testnet.2wp.iovlabs.net:19130/'
99
MAX_AMOUNT_ALLOWED_IN_SATOSHI=100000000
1010

1111
# Federation Addresses history

‎ENV_VARIABLES.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@ This table was created to guide and centralize the **environment variables** nec
4141
|ATLAS_SQS_QUEUE_URL | |'URL of the SQS FIFO queue the Atlas events are published to. Required when `ATLAS_EVENTS_ENABLED=true`; the daemon aborts at startup without it'|
4242
|AWS_REGION |`us-east-1` |'AWS region of the Atlas SQS queue' |
4343
|ATLAS_SQS_ENDPOINT |`http://localhost:4566` |'Custom SQS endpoint. Local development and tests only (LocalStack); leave empty in deployments'|
44+
|ATLAS_SQS_REQUEST_TIMEOUT_MS |`5000` |'How long to wait for the queue to answer a publish before giving up. Values of zero or less are rejected and fall back to the default'|
45+
|ATLAS_SQS_CONNECTION_TIMEOUT_MS|`2000` |'How long to wait for the connection to the queue to be established'|
46+
|ATLAS_SQS_MAX_ATTEMPTS |`3` |'Attempts per publish, retries included' |
4447

4548
### Atlas SWAP events
4649

@@ -85,6 +88,15 @@ ordered while different swaps are processed in parallel. The queue must have
8588
content based deduplication **disabled**: `MessageDeduplicationId` is the
8689
`event_id`.
8790

91+
That only works because `event_id` is **derived, never random**: it is the
92+
UUIDv5 of `swap_id`, `event_type` and the Rootstock transaction the transition
93+
was read from (`models/atlas/atlas-event-id.ts`). A reprocessed block — a
94+
restart, a re-scan — therefore derives the id it derived the first time and the
95+
queue drops the duplicate. Note the limit: the FIFO deduplication window is five
96+
minutes, so this is not idempotency by itself. What it is, is the precondition
97+
for it — an `event_id` that means the same thing on every emission is what lets
98+
Atlas deduplicate on the consumer side, where there is no window.
99+
88100
The network travels in the chain ids (`rootstock_testnet` / `bitcoin_testnet`),
89101
derived from `NETWORK`. Because a wrong network would silently contaminate the
90102
analytics database, `NETWORK` is validated when the daemon starts and the daemon
@@ -99,6 +111,22 @@ Publication happens after the status has been written to Mongo and never fails
99111
the caller: if SQS is unreachable the failure is logged at error level and block
100112
processing continues. Events lost in that window are not recovered.
101113

114+
Every publish is bounded in time. The AWS SDK sets no request timeout by
115+
default, so a queue endpoint that accepts the connection and then never answers
116+
would leave the publish pending forever — and because `RskChainSyncService`
117+
commits the block pointer *before* it notifies its subscribers, and does not
118+
await them, that hang would not stall the daemon anywhere visible: it would
119+
strand the rest of that block's Bridge transactions, unwritten and unlogged, on
120+
a block already recorded as processed. `ATLAS_SQS_REQUEST_TIMEOUT_MS` and
121+
`ATLAS_SQS_CONNECTION_TIMEOUT_MS` close that, and `ATLAS_SQS_MAX_ATTEMPTS` caps
122+
the retries, so the worst case is roughly `maxAttempts * requestTimeout` plus
123+
the SDK's backoff and a hang arrives at the error branch above as a timeout.
124+
125+
One SDK detail worth keeping in mind if these are ever retuned: `requestTimeout`
126+
on its own only **logs a warning** and lets the request hang anyway. The
127+
publisher passes `throwOnRequestTimeout: true` alongside it, which is what makes
128+
the timeout actually abort.
129+
102130
Every publication, successful or not, logs one line carrying
103131
`metric: 'atlas_events_published_total'` with `status`, `flow`, `eventType` and
104132
the running `total`. **That field name is the contract with the log aggregator**

‎src/__tests__/integration/atlas-pegout-events.integration.ts‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,51 @@ describe('Integration: Atlas peg events over SQS', function () {
230230
expect(messages[0].Attributes?.MessageDeduplicationId).to.equal(event.event_id);
231231
});
232232

233+
// The test above re-sends one event object, which would pass even if the id
234+
// were random. This is the case that actually matters: a reprocessed block
235+
// rebuilds the event from scratch, and only a derived event_id gives the
236+
// queue the same MessageDeduplicationId to drop it on.
237+
it('deduplicates a transition rebuilt from scratch, as a reprocessed block would', async () => {
238+
const swapId = givenSwapId();
239+
240+
await publisher.publish(
241+
PegoutAtlasEventBuilder.build(givenPegout(swapId, PegoutStatuses.RECEIVED))!,
242+
);
243+
await publisher.publish(
244+
PegoutAtlasEventBuilder.build(givenPegout(swapId, PegoutStatuses.RECEIVED))!,
245+
);
246+
247+
const messages = await drain(10);
248+
expect(messages).to.have.length(1);
249+
expectValid(parse(messages[0]));
250+
});
251+
252+
// The other half of the contract: deduplication must not swallow the genuine
253+
// transitions that follow, which share the swap_id and differ by event type.
254+
it('keeps the later transitions of the same peg-out', async () => {
255+
const swapId = givenSwapId();
256+
257+
await publisher.publish(
258+
PegoutAtlasEventBuilder.build(givenPegout(swapId, PegoutStatuses.RECEIVED))!,
259+
);
260+
await publisher.publish(
261+
PegoutAtlasEventBuilder.build(givenPegout(swapId, PegoutStatuses.RECEIVED))!,
262+
);
263+
await publisher.publish(
264+
PegoutAtlasEventBuilder.build(
265+
givenPegout(swapId, PegoutStatuses.WAITING_FOR_CONFIRMATION),
266+
)!,
267+
);
268+
269+
const messages = await drain(10, 2);
270+
const received = messages.map(parse);
271+
received.forEach(expectValid);
272+
expect(received.map(event => event.event_type)).to.eql([
273+
AtlasEventType.SWAP_CREATED,
274+
AtlasEventType.SWAP_PENDING,
275+
]);
276+
});
277+
233278
it('delivers a rejected peg-out as a single message', async () => {
234279
const swapId = givenSwapId();
235280
const rejected = PegoutAtlasEventBuilder.build(
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import {expect} from '@loopback/testlab';
2+
import {atlasEventId} from '../../../../models/atlas/atlas-event-id';
3+
import {AtlasEventType} from '../../../../models/atlas/atlas-event.model';
4+
5+
/** The `event_id` pattern of schemas/atlas-swap-event.schema.json. */
6+
const UUID_PATTERN = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/;
7+
/** Version 5 in the 13th nibble, RFC 4122 variant in the 17th. */
8+
const UUID_V5_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-5[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
9+
10+
const SWAP_ID = '0x8e0b47b0c60f7e02b41ee1b7d4f0d4e3f9a1c2b3d4e5f60718293a4b5c6d7e8f';
11+
const RSK_TX_HASH = '0x5b2f1a0c9d8e7f60514233a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8';
12+
13+
describe('Model: atlasEventId', () => {
14+
15+
it('derives the same id for the same event, which is what makes deduplication work', () => {
16+
const first = atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, RSK_TX_HASH);
17+
const second = atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, RSK_TX_HASH);
18+
19+
expect(first).to.equal(second);
20+
});
21+
22+
it('produces an id the schema accepts', () => {
23+
const id = atlasEventId(SWAP_ID, AtlasEventType.SWAP_COMPLETED, RSK_TX_HASH);
24+
25+
expect(id).to.match(UUID_PATTERN);
26+
});
27+
28+
// A consumer reading the version nibble is entitled to find a real one.
29+
it('produces a well formed version 5 uuid, not a hash cut into shape', () => {
30+
const id = atlasEventId(SWAP_ID, AtlasEventType.SWAP_COMPLETED, RSK_TX_HASH);
31+
32+
expect(id).to.match(UUID_V5_PATTERN);
33+
});
34+
35+
it('separates the transitions of one swap', () => {
36+
const ids = [
37+
AtlasEventType.SWAP_CREATED,
38+
AtlasEventType.SWAP_PENDING,
39+
AtlasEventType.SWAP_COMPLETED,
40+
AtlasEventType.SWAP_REJECTED,
41+
].map(eventType => atlasEventId(SWAP_ID, eventType, RSK_TX_HASH));
42+
43+
expect(new Set(ids).size).to.equal(ids.length);
44+
});
45+
46+
it('separates two swaps reporting the same transition', () => {
47+
const other = `${SWAP_ID.slice(0, -1)}0`;
48+
49+
expect(atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, RSK_TX_HASH))
50+
.to.not.equal(atlasEventId(other, AtlasEventType.SWAP_CREATED, RSK_TX_HASH));
51+
});
52+
53+
// The batched peg-outs of one Bridge transaction differ only by the index the
54+
// processor appends to `rskTxHash`, so that part has to reach the digest.
55+
it('separates two transitions differing only by the rsk transaction', () => {
56+
const batched = `${RSK_TX_HASH}___1`;
57+
58+
expect(atlasEventId(SWAP_ID, AtlasEventType.SWAP_COMPLETED, RSK_TX_HASH))
59+
.to.not.equal(atlasEventId(SWAP_ID, AtlasEventType.SWAP_COMPLETED, batched));
60+
});
61+
62+
// Same reasoning as normalizeSwapId: one transaction spelled two ways is one
63+
// event, and must not be published twice under two ids.
64+
it('ignores the spelling of the hashes it is given', () => {
65+
const id = atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, RSK_TX_HASH);
66+
67+
expect(atlasEventId(SWAP_ID.toUpperCase().replace('0X', '0x'), AtlasEventType.SWAP_CREATED, ` ${RSK_TX_HASH} `))
68+
.to.equal(id);
69+
});
70+
71+
it('still derives an id when there is no rsk transaction to name', () => {
72+
const id = atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, undefined);
73+
74+
expect(id).to.match(UUID_PATTERN);
75+
expect(atlasEventId(SWAP_ID, AtlasEventType.SWAP_CREATED, null)).to.equal(id);
76+
});
77+
78+
});

‎src/__tests__/unit/services/atlas/pegin-atlas-event.builder.unit.ts‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,11 +344,34 @@ describe('Service: PeginAtlasEventBuilder', () => {
344344
expect(PeginAtlasEventBuilder.build(pegin, {})).to.be.empty();
345345
});
346346

347-
it('generates a distinct event_id per event', () => {
348-
const first = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
349-
const second = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
347+
describe('event_id', () => {
350348

351-
expect(first[0].event_id).to.not.equal(second[0].event_id);
349+
// Same contract as the peg-out builder: the id is the deduplication key, so
350+
// rebuilding the same peg-in has to derive the same ids in the same order.
351+
it('derives the same ids when the same peg-in is rebuilt', () => {
352+
const first = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
353+
const second = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
354+
355+
expect(first.map(event => event.event_id)).to.eql(second.map(event => event.event_id));
356+
});
357+
358+
// Both events of a peg-in come from one Rootstock transaction, which leaves
359+
// the event type as the only thing separating them.
360+
it('gives the two events of one peg-in distinct ids', () => {
361+
const [created, completed] = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
362+
363+
expect(created.event_id).to.not.equal(completed.event_id);
364+
});
365+
366+
it('gives two peg-ins reporting the same transition distinct ids', () => {
367+
const other = givenPegin(PeginStatus.LOCKED);
368+
other.btcTxId = `${btcTxId.slice(0, -1)}1`;
369+
370+
const [created] = PeginAtlasEventBuilder.build(givenPegin(PeginStatus.LOCKED), {});
371+
const [otherCreated] = PeginAtlasEventBuilder.build(other, {});
372+
373+
expect(created.event_id).to.not.equal(otherCreated.event_id);
374+
});
352375
});
353376

354377
it('throws when NETWORK is not configured instead of guessing the network', () => {

‎src/__tests__/unit/services/atlas/pegout-atlas-event.builder.unit.ts‎

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,48 @@ describe('Service: PegoutAtlasEventBuilder', () => {
353353
});
354354
});
355355

356-
it('generates a distinct event_id per event', () => {
357-
const first = PegoutAtlasEventBuilder.build(givenPegout({status: PegoutStatuses.RECEIVED}))!;
358-
const second = PegoutAtlasEventBuilder.build(givenPegout({status: PegoutStatuses.RECEIVED}))!;
359-
expect(first.event_id).to.not.equal(second.event_id);
356+
describe('event_id', () => {
357+
358+
// The publisher sends it as the MessageDeduplicationId, so a reprocessed
359+
// block has to derive the id it derived the first time or the queue has
360+
// nothing to deduplicate on.
361+
it('derives the same id when the same transition is rebuilt', () => {
362+
const first = PegoutAtlasEventBuilder.build(givenPegout({status: PegoutStatuses.RECEIVED}))!;
363+
const second = PegoutAtlasEventBuilder.build(givenPegout({status: PegoutStatuses.RECEIVED}))!;
364+
365+
expect(first.event_id).to.equal(second.event_id);
366+
});
367+
368+
it('gives each transition of one peg-out its own id', () => {
369+
const received = PegoutAtlasEventBuilder.build(givenPegout({
370+
status: PegoutStatuses.RECEIVED,
371+
}))!;
372+
const waiting = PegoutAtlasEventBuilder.build(givenPegout({
373+
status: PegoutStatuses.WAITING_FOR_CONFIRMATION,
374+
}))!;
375+
376+
expect(received.event_id).to.not.equal(waiting.event_id);
377+
});
378+
379+
// Batched peg-outs share the Bridge transaction and are told apart by the
380+
// index the processor appends to rskTxHash.
381+
it('gives the batched peg-outs of one Bridge transaction their own ids', () => {
382+
const first = PegoutAtlasEventBuilder.build(givenPegout({
383+
status: PegoutStatuses.WAITING_FOR_CONFIRMATION,
384+
rskTxHash: `${originatingRskTxHash}___0`,
385+
}))!;
386+
const second = PegoutAtlasEventBuilder.build(givenPegout({
387+
status: PegoutStatuses.WAITING_FOR_CONFIRMATION,
388+
rskTxHash: `${originatingRskTxHash}___1`,
389+
}))!;
390+
391+
expect(first.event_id).to.not.equal(second.event_id);
392+
});
393+
394+
it('derives an id the schema accepts', () => {
395+
expectValidAgainstSchema(
396+
PegoutAtlasEventBuilder.build(givenPegout({status: PegoutStatuses.RECEIVED})),
397+
);
398+
});
360399
});
361400
});

‎src/__tests__/unit/services/atlas/sqs-atlas-event-publisher.unit.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as net from 'net';
12
import {Application} from '@loopback/core';
23
import {expect, sinon} from '@loopback/testlab';
34
import {SQSClient} from '@aws-sdk/client-sqs';
@@ -12,6 +13,7 @@ import {isAtlasEventsEnabled} from '../../../../services/atlas/atlas-event-publi
1213
import {
1314
SqsAtlasEventPublisher,
1415
assertQueueUrlConfigured,
16+
positiveIntFromEnv,
1517
} from '../../../../services/atlas/sqs-atlas-event-publisher';
1618
import {NoopAtlasEventPublisher} from '../../../../services/atlas/noop-atlas-event-publisher';
1719
import {DependencyInjectionHandler} from '../../../../dependency-injection-handler';
@@ -103,6 +105,87 @@ describe('Service: SqsAtlasEventPublisher', () => {
103105
await publisher.publish(event);
104106
});
105107

108+
describe('request timeout', () => {
109+
const originalCredentials = {
110+
keyId: process.env.AWS_ACCESS_KEY_ID,
111+
secret: process.env.AWS_SECRET_ACCESS_KEY,
112+
requestTimeout: process.env.ATLAS_SQS_REQUEST_TIMEOUT_MS,
113+
maxAttempts: process.env.ATLAS_SQS_MAX_ATTEMPTS,
114+
};
115+
let server: net.Server;
116+
const connections: net.Socket[] = [];
117+
118+
/**
119+
* Accepts the connection and never answers, which is the failure a
120+
* connection timeout alone does not catch.
121+
*/
122+
before(async () => {
123+
server = net.createServer(socket => {
124+
// Kept only so the teardown can destroy them: the SDK leaves the socket
125+
// alive, and `close()` waits for every connection before it fires.
126+
connections.push(socket);
127+
});
128+
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve));
129+
});
130+
131+
after(async () => {
132+
connections.forEach(socket => socket.destroy());
133+
await new Promise<void>(resolve => { server.close(() => resolve()); });
134+
restore('AWS_ACCESS_KEY_ID', originalCredentials.keyId);
135+
restore('AWS_SECRET_ACCESS_KEY', originalCredentials.secret);
136+
restore('ATLAS_SQS_REQUEST_TIMEOUT_MS', originalCredentials.requestTimeout);
137+
restore('ATLAS_SQS_MAX_ATTEMPTS', originalCredentials.maxAttempts);
138+
});
139+
140+
// Without a request timeout this publish never settles. The sync service
141+
// commits the block pointer before it notifies its subscribers and does not
142+
// await them, so the hang would strand the rest of the block silently
143+
// rather than stall the daemon somewhere visible.
144+
it('gives up on a queue that accepts the connection and never answers', async () => {
145+
const {port} = server.address() as net.AddressInfo;
146+
process.env.ATLAS_SQS_ENDPOINT = `http://127.0.0.1:${port}`;
147+
process.env.ATLAS_SQS_REQUEST_TIMEOUT_MS = '150';
148+
process.env.ATLAS_SQS_MAX_ATTEMPTS = '1';
149+
process.env.AWS_ACCESS_KEY_ID = 'test';
150+
process.env.AWS_SECRET_ACCESS_KEY = 'test';
151+
const publisher = new SqsAtlasEventPublisher();
152+
153+
const startedAt = Date.now();
154+
await publisher.publish(event, 'pegout');
155+
const elapsed = Date.now() - startedAt;
156+
157+
expect(elapsed).to.be.lessThan(5000);
158+
expect(publisher.metrics.total('failure', event.event_type, 'pegout')).to.equal(1);
159+
expect(publisher.metrics.total('success', event.event_type, 'pegout')).to.equal(0);
160+
publisher.destroy();
161+
});
162+
});
163+
164+
describe('timeout configuration', () => {
165+
const NAME = 'ATLAS_SQS_TEST_VALUE';
166+
167+
afterEach(() => delete process.env[NAME]);
168+
169+
it('takes a positive value from the environment', () => {
170+
process.env[NAME] = '750';
171+
172+
expect(positiveIntFromEnv(NAME, 5000)).to.equal(750);
173+
});
174+
175+
it('falls back when the variable is absent', () => {
176+
expect(positiveIntFromEnv(NAME, 5000)).to.equal(5000);
177+
});
178+
179+
// Zero and negative are how the SDK spells "wait forever", which is the
180+
// failure the timeout exists to prevent: they are not valid settings.
181+
it('falls back rather than accepting a value that disables the timeout', () => {
182+
for (const value of ['0', '-1', '-4000', '', 'soon', 'NaN']) {
183+
process.env[NAME] = value;
184+
expect(positiveIntFromEnv(NAME, 5000)).to.equal(5000);
185+
}
186+
});
187+
});
188+
106189
describe('publication metric', () => {
107190
it('records a success when SQS accepts the message', async () => {
108191
sandbox.stub(SQSClient.prototype, 'send').resolves({MessageId: 'id'} as never);

‎src/__tests__/unit/services/btc-last-block.service.unit.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ describe('Service: BitcoinService', () => {
5050

5151
it('Verify ${process.env.BLOCKBOOK_URL} configuration', async () => {
5252
const nodeHost = process.env.BLOCKBOOK_URL;
53-
sinon.assert.match(nodeHost, 'https://blockbook-01.testnet.2wp.iovlabs.net:19130/');
53+
sinon.assert.match(nodeHost, 'https://blockbook.testnet.2wp.iovlabs.net:19130/');
5454
});
5555

5656
});

0 commit comments

Comments
 (0)