Skip to content

Commit

Permalink
fix: reduce getSingleTrueBit() calls
Browse files Browse the repository at this point in the history
  • Loading branch information
twoeths committed Oct 30, 2024
1 parent 558ec2f commit 7d3d255
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 21 deletions.
16 changes: 8 additions & 8 deletions packages/beacon-node/src/api/impl/beacon/pool/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,16 +99,16 @@ export function getBeaconPoolApi({
// when a validator is configured with multiple beacon node urls, this attestation data may come from another beacon node
// and the block hasn't been in our forkchoice since we haven't seen / processing that block
// see https://github.com/ChainSafe/lodestar/issues/5098
const {indexedAttestation, subnet, attDataRootHex, committeeIndex} = await validateGossipFnRetryUnknownRoot(
validateFn,
network,
chain,
slot,
beaconBlockRoot
);
const {indexedAttestation, subnet, attDataRootHex, committeeIndex, participationIndex} =
await validateGossipFnRetryUnknownRoot(validateFn, network, chain, slot, beaconBlockRoot);

if (network.shouldAggregate(subnet, slot)) {
const insertOutcome = chain.attestationPool.add(committeeIndex, attestation, attDataRootHex);
const insertOutcome = chain.attestationPool.add(
committeeIndex,
participationIndex,
attestation,
attDataRootHex
);
metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome});
}

Expand Down
13 changes: 8 additions & 5 deletions packages/beacon-node/src/chain/opPools/attestationPool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,12 @@ export class AttestationPool {
* - Valid committeeIndex
* - Valid data
*/
add(committeeIndex: CommitteeIndex, attestation: Attestation, attDataRootHex: RootHex): InsertOutcome {
add(
committeeIndex: CommitteeIndex,
participationIndex: number,
attestation: Attestation,
attDataRootHex: RootHex
): InsertOutcome {
const slot = attestation.data.slot;
const fork = this.config.getForkName(slot);
const lowestPermissibleSlot = this.lowestPermissibleSlot;
Expand Down Expand Up @@ -144,7 +149,7 @@ export class AttestationPool {
const aggregate = aggregateByIndex.get(committeeIndex);
if (aggregate) {
// Aggregate mutating
return aggregateAttestationInto(aggregate, attestation);
return aggregateAttestationInto(aggregate, attestation, participationIndex);
}
// Create new aggregate
aggregateByIndex.set(committeeIndex, attestationToAggregate(attestation));
Expand Down Expand Up @@ -216,9 +221,7 @@ export class AttestationPool {
/**
* Aggregate a new attestation into `aggregate` mutating it
*/
function aggregateAttestationInto(aggregate: AggregateFast, attestation: Attestation): InsertOutcome {
const bitIndex = attestation.aggregationBits.getSingleTrueBit();

function aggregateAttestationInto(aggregate: AggregateFast, attestation: Attestation, bitIndex: number): InsertOutcome {
// Should never happen, attestations are verified against this exact condition before
assert.notNull(bitIndex, "Invalid attestation in pool, not exactly one bit set");

Expand Down
2 changes: 2 additions & 0 deletions packages/beacon-node/src/chain/validation/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export type AttestationValidationResult = {
subnet: number;
attDataRootHex: RootHex;
committeeIndex: CommitteeIndex;
participationIndex: number;
};

export type AttestationOrBytes = ApiAttestation | GossipAttestation;
Expand Down Expand Up @@ -505,6 +506,7 @@ async function validateAttestationNoSignatureCheck(
signatureSet,
validatorIndex,
committeeIndex,
participationIndex: bitIndex,
};
}

Expand Down
10 changes: 8 additions & 2 deletions packages/beacon-node/src/network/processor/gossipHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -633,14 +633,20 @@ function getBatchHandlers(modules: ValidatorFnsModules, options: GossipHandlerOp
results.push(null);

// Handler
const {indexedAttestation, attDataRootHex, attestation, committeeIndex} = validationResult.result;
const {indexedAttestation, attDataRootHex, attestation, committeeIndex, participationIndex} =
validationResult.result;
metrics?.registerGossipUnaggregatedAttestation(gossipHandlerParams[i].seenTimestampSec, indexedAttestation);

try {
// Node may be subscribe to extra subnets (long-lived random subnets). For those, validate the messages
// but don't add to attestation pool, to save CPU and RAM
if (aggregatorTracker.shouldAggregate(subnet, indexedAttestation.data.slot)) {
const insertOutcome = chain.attestationPool.add(committeeIndex, attestation, attDataRootHex);
const insertOutcome = chain.attestationPool.add(
committeeIndex,
participationIndex,
attestation,
attDataRootHex
);
metrics?.opPool.attestationPoolInsertOutcome.inc({insertOutcome});
}
} catch (e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe("AttestationPool", () => {
};

let pool: AttestationPool;
const participationIndex = 0;

beforeEach(() => {
pool = new AttestationPool(config, clockStub, cutOffSecFromSlot);
Expand All @@ -52,7 +53,7 @@ describe("AttestationPool", () => {
it("add correct electra attestation", () => {
const committeeIndex = 0;
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data));
const outcome = pool.add(committeeIndex, electraAttestation, attDataRootHex);
const outcome = pool.add(committeeIndex, participationIndex, electraAttestation, attDataRootHex);

expect(outcome).equal(InsertOutcome.NewData);
expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toEqual(electraAttestation);
Expand All @@ -61,7 +62,7 @@ describe("AttestationPool", () => {
it("add correct phase0 attestation", () => {
const committeeIndex = null;
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0Attestation.data));
const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex);
const outcome = pool.add(committeeIndex, participationIndex, phase0Attestation, attDataRootHex);

expect(outcome).equal(InsertOutcome.NewData);
expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation);
Expand All @@ -74,14 +75,14 @@ describe("AttestationPool", () => {
const committeeIndex = null;
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestation.data));

expect(() => pool.add(committeeIndex, electraAttestation, attDataRootHex)).toThrow();
expect(() => pool.add(committeeIndex, participationIndex, electraAttestation, attDataRootHex)).toThrow();
expect(pool.getAggregate(electraAttestationData.slot, committeeIndex, attDataRootHex)).toBeNull();
});

it("add phase0 attestation with committee index", () => {
const committeeIndex = 0;
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0Attestation.data));
const outcome = pool.add(committeeIndex, phase0Attestation, attDataRootHex);
const outcome = pool.add(committeeIndex, participationIndex, phase0Attestation, attDataRootHex);

expect(outcome).equal(InsertOutcome.NewData);
expect(pool.getAggregate(phase0AttestationData.slot, committeeIndex, attDataRootHex)).toEqual(phase0Attestation);
Expand All @@ -99,7 +100,7 @@ describe("AttestationPool", () => {
};
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(electraAttestationDataWithPhase0Slot));

expect(() => pool.add(0, attestation, attDataRootHex)).toThrow();
expect(() => pool.add(0, participationIndex, attestation, attDataRootHex)).toThrow();
});

it("add phase0 attestation with electra slot", () => {
Expand All @@ -114,6 +115,6 @@ describe("AttestationPool", () => {
};
const attDataRootHex = toHexString(ssz.phase0.AttestationData.hashTreeRoot(phase0AttestationDataWithElectraSlot));

expect(() => pool.add(0, attestation, attDataRootHex)).toThrow();
expect(() => pool.add(0, participationIndex, attestation, attDataRootHex)).toThrow();
});
});

0 comments on commit 7d3d255

Please sign in to comment.