Skip to content
Merged
51 changes: 50 additions & 1 deletion packages/jam/jamnp-s/tasks/ticket-distribution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ describe("TicketDistributionTask", () => {
peer1.ticketTask.maintainDistribution();
await tick();

// Self receives the ticket (via onTicketReceived -> addTicket)
// Self receives the ticket (via onTicketReceived -> addTicket, no callback set)
assert.strictEqual(self.receivedTickets.length, 1);

// Self should re-distribute to peer2 (and peer1, but peer1 already has it)
Expand All @@ -272,4 +272,53 @@ describe("TicketDistributionTask", () => {
assert.strictEqual(peer2.receivedTickets.length, 1);
assert.deepStrictEqual(peer2.receivedTickets[0].ticket, ticket);
});

it("should NOT redistribute ticket if validation callback returns false", async () => {
const self = await init("self");
const peer1 = await init("peer1");
const peer2 = await init("peer2");

self.openConnection(peer1);
self.openConnection(peer2);
await tick();

// Validation always rejects
self.ticketTask.setOnTicketReceived(async () => false);

const ticket = createTestTicket(0);
peer1.ticketTask.addTicket(TEST_EPOCH, ticket);
peer1.ticketTask.maintainDistribution();
await tick();

// self.addTicket was NOT called (callback returned false), so nothing to redistribute
assert.strictEqual(self.receivedTickets.length, 0);
self.ticketTask.maintainDistribution();
await tick();
assert.strictEqual(peer2.receivedTickets.length, 0);
});

it("should redistribute ticket if validation callback returns true", async () => {
const self = await init("self");
const peer1 = await init("peer1");
const peer2 = await init("peer2");

self.openConnection(peer1);
self.openConnection(peer2);
await tick();

// Validation always accepts
self.ticketTask.setOnTicketReceived(async () => true);

const ticket = createTestTicket(0);
peer1.ticketTask.addTicket(TEST_EPOCH, ticket);
peer1.ticketTask.maintainDistribution();
await tick();

// self.addTicket WAS called (callback returned true)
assert.strictEqual(self.receivedTickets.length, 1);
self.ticketTask.maintainDistribution();
await tick();
assert.strictEqual(peer2.receivedTickets.length, 1);
assert.deepStrictEqual(peer2.receivedTickets[0].ticket, ticket);
});
});
43 changes: 39 additions & 4 deletions packages/jam/jamnp-s/tasks/ticket-distribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,15 @@ export class TicketDistributionTask {
* Deduplicates tickets based on signature.
*/
addTicket(epochIndex: Epoch, ticket: SignedTicket) {
// Check if epoch changed - if so, clear old tickets
if (this.currentEpoch !== null && this.currentEpoch !== epochIndex) {
// Drop tickets for older epochs (can happen when a delayed validation callback completes
// after the epoch has already advanced — accepting it would roll back currentEpoch).
if (this.currentEpoch !== null && epochIndex < this.currentEpoch) {
logger.warn`[addTicket] Ignoring ticket for old epoch ${epochIndex} (current: ${this.currentEpoch})`;
return;
}
Comment thread
tomusdrw marked this conversation as resolved.

// Epoch advanced — clear old tickets
if (this.currentEpoch !== null && epochIndex > this.currentEpoch) {
logger.log`[addTicket] Epoch changed from ${this.currentEpoch} to ${epochIndex}, clearing ${this.pendingTickets.length} old tickets`;
this.pendingTickets = [];
// Note: We don't need to clear aux data for all peers here.
Expand Down Expand Up @@ -144,9 +151,37 @@ export class TicketDistributionTask {
}
}

private onTicketReceivedCallback: ((epochIndex: Epoch, ticket: SignedTicket) => Promise<boolean>) | null = null;

/**
* Register a callback that validates a received ticket.
* The ticket is only added to the redistribution pool if the callback returns `true`.
* This prevents redistribution of invalid tickets (e.g. those with a tampered `attempt` field).
*/
setOnTicketReceived(cb: (epochIndex: Epoch, ticket: SignedTicket) => Promise<boolean>) {
this.onTicketReceivedCallback = cb;
}
Comment thread
tomusdrw marked this conversation as resolved.

private onTicketReceived(epochIndex: Epoch, ticket: SignedTicket) {
logger.trace`Received ticket for epoch ${epochIndex}, attempt ${ticket.attempt}`;
// Add to pending queue for potential re-distribution
this.addTicket(epochIndex, ticket);
if (this.onTicketReceivedCallback !== null) {
// Validate first; only redistribute if valid to avoid spreading tampered tickets.
// Wrap with Promise.resolve().then() to catch both sync throws and async rejections.
const cb = this.onTicketReceivedCallback;
Promise.resolve()
.then(() => cb(epochIndex, ticket))
.then((isValid) => {
if (isValid) {
this.addTicket(epochIndex, ticket);
Comment thread
tomusdrw marked this conversation as resolved.
} else {
logger.warn`Dropping invalid ticket for epoch ${epochIndex} (validation failed)`;
}
})
.catch((error) => {
logger.error`Error validating ticket for epoch ${epochIndex}, attempt ${ticket.attempt}: ${error}`;
});
} else {
this.addTicket(epochIndex, ticket);
}
}
}
204 changes: 204 additions & 0 deletions packages/workers/block-authorship/generator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import {
type ValidatorIndex,
ValidatorKeys,
} from "@typeberry/block";
import { SignedTicket, Ticket, tryAsTicketAttempt } from "@typeberry/block/tickets.js";
import { Bytes, BytesBlob } from "@typeberry/bytes";
import { asKnownSize, FixedSizeArray } from "@typeberry/collections";
import { tinyChainSpec } from "@typeberry/config";
import {
BANDERSNATCH_KEY_BYTES,
BANDERSNATCH_PROOF_BYTES,
BANDERSNATCH_VRF_SIGNATURE_BYTES,
BLS_KEY_BYTES,
ED25519_KEY_BYTES,
Expand Down Expand Up @@ -222,6 +224,208 @@ describe("Generator", () => {
deepEqual(block, expectedBlock);
});

it("should include sorted tickets during contest period", async () => {
// tinyChainSpec: contestLength = 10, so slot 1 is in contest period (1 < 10)
const state = createMockState(0);
const blocksDb = createMockBlocksDb(MOCK_PARENT_HASH);
const statesDb = createMockStatesDb(state);

const generator = new Generator(tinyChainSpec, bandersnatch, keccakHasher, blake2b, blocksDb, statesDb);

// Create two tickets with different signatures
const sig1 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig1.raw[0] = 1;
const sig2 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig2.raw[0] = 2;

const ticket1 = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig1.asOpaque(),
});
const ticket2 = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig2.asOpaque(),
});

// ticket2 gets smaller ID (0x01...) and ticket1 gets larger ID (0x02...)
// so the sorted order should be [ticket2, ticket1]
const id1 = Bytes.fill(HASH_SIZE, 0x02).asOpaque<EntropyHash>();
const id2 = Bytes.fill(HASH_SIZE, 0x01).asOpaque<EntropyHash>();

const validatorIndex = tryAsValidatorIndex(0);
// Slot 1 is in contest period (1 < contestLength=10)
const timeSlot = tryAsTimeSlot(1);

// IDs are now pre-computed before passing to nextBlock
const block = await generator.nextBlock(validatorIndex, MOCK_BANDERSNATCH_SECRET, MOCK_SEAL_PAYLOAD, timeSlot, [
{ ticket: ticket1, id: id1 },
{ ticket: ticket2, id: id2 },
]);

// Tickets should be sorted by ID ascending: ticket2 (id=0x01) before ticket1 (id=0x02)
const tickets = block.extrinsic.tickets as unknown as SignedTicket[];
deepEqual(tickets.length, 2);
deepEqual(tickets[0].signature, sig2.asOpaque());
deepEqual(tickets[1].signature, sig1.asOpaque());
});

it("should exclude tickets outside contest period", async () => {
// tinyChainSpec: contestLength = 10, epochLength = 12
// Slot 10 is outside contest period (10 >= 10)
const state = createMockState(9);
const blocksDb = createMockBlocksDb(MOCK_PARENT_HASH);
const statesDb = createMockStatesDb(state);

const generator = new Generator(tinyChainSpec, bandersnatch, keccakHasher, blake2b, blocksDb, statesDb);

const sig1 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
const ticket1 = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig1.asOpaque(),
});

const validatorIndex = tryAsValidatorIndex(0);
// Slot 10 is NOT in contest period (10 >= contestLength=10)
const timeSlot = tryAsTimeSlot(10);

const mockId = Bytes.fill(HASH_SIZE, 0x01).asOpaque<EntropyHash>();
const block = await generator.nextBlock(validatorIndex, MOCK_BANDERSNATCH_SECRET, MOCK_SEAL_PAYLOAD, timeSlot, [
{ ticket: ticket1, id: mockId },
]);

// No tickets should be included outside contest period
const tickets = block.extrinsic.tickets as unknown as SignedTicket[];
deepEqual(tickets.length, 0);
});

it("should filter out tickets already in ticketsAccumulator", async () => {
// Build a state that already has ticket with id=0x01 in its accumulator
const accumulatedId = Bytes.fill(HASH_SIZE, 0x01).asOpaque<EntropyHash>();
const accumulatedTicket = Ticket.create({
id: accumulatedId,
attempt: tryAsTicketAttempt(0),
});

const state = {
...createMockState(0),
ticketsAccumulator: asKnownSize([accumulatedTicket]),
};
const blocksDb = createMockBlocksDb(MOCK_PARENT_HASH);
const statesDb = createMockStatesDb(state);

const generator = new Generator(tinyChainSpec, bandersnatch, keccakHasher, blake2b, blocksDb, statesDb);

const sig1 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig1.raw[0] = 1;
const sig2 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig2.raw[0] = 2;

const ticketAlreadyAccumulated = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig1.asOpaque(),
});
const ticketNew = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig2.asOpaque(),
});

// id=0x01 is already in accumulator, id=0x02 is new
const idAccumulated = Bytes.fill(HASH_SIZE, 0x01).asOpaque<EntropyHash>();
const idNew = Bytes.fill(HASH_SIZE, 0x02).asOpaque<EntropyHash>();

const validatorIndex = tryAsValidatorIndex(0);
const timeSlot = tryAsTimeSlot(1); // inside contest period

const block = await generator.nextBlock(validatorIndex, MOCK_BANDERSNATCH_SECRET, MOCK_SEAL_PAYLOAD, timeSlot, [
{ ticket: ticketAlreadyAccumulated, id: idAccumulated },
{ ticket: ticketNew, id: idNew },
]);

// Only the new ticket (not in accumulator) should be included
const tickets = block.extrinsic.tickets as unknown as SignedTicket[];
deepEqual(tickets.length, 1);
deepEqual(tickets[0].signature, sig2.asOpaque());
});

it("should deduplicate tickets by ID", async () => {
const state = createMockState(0);
const blocksDb = createMockBlocksDb(MOCK_PARENT_HASH);
const statesDb = createMockStatesDb(state);

const generator = new Generator(tinyChainSpec, bandersnatch, keccakHasher, blake2b, blocksDb, statesDb);

const sig1 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig1.raw[0] = 1;
const sig2 = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig2.raw[0] = 2;

// Two different SignedTicket objects but with the same ID (e.g. duplicate from reorg)
const ticketA = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig1.asOpaque(),
});
const ticketB = SignedTicket.create({
attempt: tryAsTicketAttempt(0),
signature: sig2.asOpaque(),
});
const duplicateId = Bytes.fill(HASH_SIZE, 0x05).asOpaque<EntropyHash>();

const validatorIndex = tryAsValidatorIndex(0);
const timeSlot = tryAsTimeSlot(1);

const block = await generator.nextBlock(validatorIndex, MOCK_BANDERSNATCH_SECRET, MOCK_SEAL_PAYLOAD, timeSlot, [
{ ticket: ticketA, id: duplicateId },
{ ticket: ticketB, id: duplicateId }, // same ID — should be deduplicated
]);

const tickets = block.extrinsic.tickets as unknown as SignedTicket[];
deepEqual(tickets.length, 1);
// First occurrence is kept after sort (both have same ID, ticketA comes first)
deepEqual(tickets[0].signature, sig1.asOpaque());
});

it("should include at most maxTicketsPerExtrinsic tickets", async () => {
// tinyChainSpec.maxTicketsPerExtrinsic = 3
const state = createMockState(0);
const blocksDb = createMockBlocksDb(MOCK_PARENT_HASH);
const statesDb = createMockStatesDb(state);

const generator = new Generator(tinyChainSpec, bandersnatch, keccakHasher, blake2b, blocksDb, statesDb);

// Create 4 tickets — only 3 should be included (lowest IDs win)
const makeTicket = (sigByte: number, idByte: number) => {
const sig = Bytes.zero(BANDERSNATCH_PROOF_BYTES);
sig.raw[0] = sigByte;
return {
ticket: SignedTicket.create({ attempt: tryAsTicketAttempt(0), signature: sig.asOpaque() }),
id: Bytes.fill(HASH_SIZE, idByte).asOpaque<EntropyHash>(),
sig: sig.asOpaque(),
};
};

const t1 = makeTicket(1, 0x01); // lowest ID
const t2 = makeTicket(2, 0x02);
const t3 = makeTicket(3, 0x03);
const t4 = makeTicket(4, 0x04); // highest ID — should be excluded

const validatorIndex = tryAsValidatorIndex(0);
const timeSlot = tryAsTimeSlot(1);

const block = await generator.nextBlock(validatorIndex, MOCK_BANDERSNATCH_SECRET, MOCK_SEAL_PAYLOAD, timeSlot, [
{ ticket: t4.ticket, id: t4.id }, // pass out-of-order to verify sorting
{ ticket: t2.ticket, id: t2.id },
{ ticket: t3.ticket, id: t3.id },
{ ticket: t1.ticket, id: t1.id },
]);

const tickets = block.extrinsic.tickets as unknown as SignedTicket[];
deepEqual(tickets.length, 3); // maxTicketsPerExtrinsic = 3
// Should include the 3 lowest IDs, sorted ascending
deepEqual(tickets[0].signature, t1.sig);
deepEqual(tickets[1].signature, t2.sig);
deepEqual(tickets[2].signature, t3.sig);
});

it("should create block with epoch marker at epoch boundary", async () => {
// tinyChainSpec.epochLength = 12, so:
// - timeslot 11 is last slot of epoch 0
Expand Down
Loading
Loading