Skip to content
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.

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

Clarify #3977 with unbounded uint issue #4018

Merged
merged 1 commit into from
May 13, 2022
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
Expand Up @@ -2,7 +2,7 @@ import {MAX_VALIDATORS_PER_COMMITTEE} from "@chainsafe/lodestar-params";
import {phase0} from "@chainsafe/lodestar-types";
import {CachedBeaconStateAllForks} from "../../types";
import {verifySignatureSet} from "../../util";
import {getIndexedAttestationBnSignatureSet, getIndexedAttestationSignatureSet} from "../signatureSets";
import {getIndexedAttestationBigintSignatureSet, getIndexedAttestationSignatureSet} from "../signatureSets";

/**
* Check if `indexedAttestation` has sorted and unique indices and a valid aggregate signature.
Expand All @@ -23,17 +23,17 @@ export function isValidIndexedAttestation(
}
}

export function isValidIndexedAttestationBn(
export function isValidIndexedAttestationBigint(
state: CachedBeaconStateAllForks,
indexedAttestation: phase0.IndexedAttestationBn,
indexedAttestation: phase0.IndexedAttestationBigint,
verifySignature: boolean
): boolean {
if (!isValidIndexedAttestationIndices(state, indexedAttestation.attestingIndices)) {
return false;
}

if (verifySignature) {
return verifySignatureSet(getIndexedAttestationBnSignatureSet(state, indexedAttestation));
return verifySignatureSet(getIndexedAttestationBigintSignatureSet(state, indexedAttestation));
} else {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {ForkName} from "@chainsafe/lodestar-params";
import {isSlashableValidator, isSlashableAttestationData, getAttesterSlashableIndices} from "../../util";
import {CachedBeaconStateAllForks} from "../../types";
import {slashValidatorAllForks} from "./slashValidator";
import {isValidIndexedAttestationBn} from "./isValidIndexedAttestation";
import {isValidIndexedAttestationBigint} from "./isValidIndexedAttestation";

/**
* Process an AttesterSlashing operation. Initiates the exit of a validator, decreases the balance of the slashed
Expand Down Expand Up @@ -49,8 +49,11 @@ export function assertValidAttesterSlashing(
throw new Error("AttesterSlashing is not slashable");
}

// In state transition, AttesterSlashing attestations are only partially validated. Their slot and epoch could
// be higher than the clock and the slashing would still be valid. Same applies to attestation data index, which
// can be any arbitrary value. Must use bigint variants to hash correctly to all possible values
for (const [i, attestation] of [attestation1, attestation2].entries()) {
if (!isValidIndexedAttestationBn(state, attestation, verifySignatures)) {
if (!isValidIndexedAttestationBigint(state, attestation, verifySignatures)) {
throw new Error(`AttesterSlashing attestation${i} is invalid`);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export function assertValidProposerSlashing(
}

// verify headers are different
if (ssz.phase0.BeaconBlockHeaderBn.equals(header1, header2)) {
if (ssz.phase0.BeaconBlockHeaderBigint.equals(header1, header2)) {
throw new Error("ProposerSlashing headers are equal");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ export function getAttesterSlashingSignatureSets(
attesterSlashing: phase0.AttesterSlashing
): ISignatureSet[] {
return [attesterSlashing.attestation1, attesterSlashing.attestation2].map((attestation) =>
getIndexedAttestationBnSignatureSet(state, attestation)
getIndexedAttestationBigintSignatureSet(state, attestation)
);
}

export function getIndexedAttestationBnSignatureSet(
export function getIndexedAttestationBigintSignatureSet(
state: CachedBeaconStateAllForks,
indexedAttestation: phase0.IndexedAttestationBn
indexedAttestation: phase0.IndexedAttestationBigint
): ISignatureSet {
const {index2pubkey} = state.epochCtx;
const slot = computeStartSlotAtEpoch(Number(indexedAttestation.data.target.epoch as bigint));
Expand All @@ -34,7 +34,7 @@ export function getIndexedAttestationBnSignatureSet(
return {
type: SignatureSetType.aggregate,
pubkeys: indexedAttestation.attestingIndices.map((i) => index2pubkey[i]),
signingRoot: computeSigningRoot(ssz.phase0.AttestationDataBn, indexedAttestation.data, domain),
signingRoot: computeSigningRoot(ssz.phase0.AttestationDataBigint, indexedAttestation.data, domain),
signature: indexedAttestation.signature,
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ export function getProposerSlashingSignatureSets(
const {epochCtx} = state;
const pubkey = epochCtx.index2pubkey[proposerSlashing.signedHeader1.message.proposerIndex];

// In state transition, ProposerSlashing headers are only partially validated. Their slot could be higher than the
// clock and the slashing would still be valid. Must use bigint variants to hash correctly to all possible values
return [proposerSlashing.signedHeader1, proposerSlashing.signedHeader2].map(
(signedHeader): ISignatureSet => {
const domain = state.config.getDomain(DOMAIN_BEACON_PROPOSER, Number(signedHeader.message.slot as bigint));

return {
type: SignatureSetType.single,
pubkey,
signingRoot: computeSigningRoot(ssz.phase0.BeaconBlockHeaderBn, signedHeader.message, domain),
signingRoot: computeSigningRoot(ssz.phase0.BeaconBlockHeaderBigint, signedHeader.message, domain),
signature: signedHeader.signature,
};
}
Expand Down
7 changes: 5 additions & 2 deletions packages/beacon-state-transition/src/util/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ import {phase0, Slot, ssz, ValidatorIndex} from "@chainsafe/lodestar-types";
/**
* Check if [[data1]] and [[data2]] are slashable according to Casper FFG rules.
*/
export function isSlashableAttestationData(data1: phase0.AttestationDataBn, data2: phase0.AttestationDataBn): boolean {
export function isSlashableAttestationData(
data1: phase0.AttestationDataBigint,
data2: phase0.AttestationDataBigint
): boolean {
return (
// Double vote
(!ssz.phase0.AttestationDataBn.equals(data1, data2) && data1.target.epoch === data2.target.epoch) ||
(!ssz.phase0.AttestationDataBigint.equals(data1, data2) && data1.target.epoch === data2.target.epoch) ||
// Surround vote
(data1.source.epoch < data2.source.epoch && data2.target.epoch < data1.target.epoch)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export function getBlockPhase0(
const startIndex = attesterSlashingStartIndex + i * bitsLen * exitedIndexStep;
const attestingIndices = linspace(startIndex, bitsLen, exitedIndexStep);

const attData: phase0.AttestationDataBn = {
const attData: phase0.AttestationDataBigint = {
slot: BigInt(attSlot),
index: BigInt(0),
beaconBlockRoot: rootA,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,12 @@ interface IBlockProposerData {

function getMockProposerSlashings(data1: IBlockProposerData, data2: IBlockProposerData): phase0.ProposerSlashing {
return {
signedHeader1: getMockSignedBeaconBlockHeaderBn(data1),
signedHeader2: getMockSignedBeaconBlockHeaderBn(data2),
signedHeader1: getMockSignedBeaconBlockHeaderBigint(data1),
signedHeader2: getMockSignedBeaconBlockHeaderBigint(data2),
};
}

function getMockSignedBeaconBlockHeaderBn(data: IBlockProposerData): phase0.SignedBeaconBlockHeaderBn {
function getMockSignedBeaconBlockHeaderBigint(data: IBlockProposerData): phase0.SignedBeaconBlockHeaderBigint {
return {
message: {
slot: BigInt(0),
Expand All @@ -118,10 +118,10 @@ function getMockAttesterSlashings(data1: IIndexAttestationData, data2: IIndexAtt
};
}

function getMockIndexAttestationBn(data: IIndexAttestationData): phase0.IndexedAttestationBn {
function getMockIndexAttestationBn(data: IIndexAttestationData): phase0.IndexedAttestationBigint {
return {
attestingIndices: data.attestingIndices,
data: getAttestationDataBn(),
data: getAttestationDataBigint(),
signature: data.signature,
};
}
Expand All @@ -136,7 +136,7 @@ function getAttestationData(): phase0.AttestationData {
};
}

function getAttestationDataBn(): phase0.AttestationDataBn {
function getAttestationDataBigint(): phase0.AttestationDataBigint {
return {
slot: BigInt(0),
index: BigInt(0),
Expand Down
24 changes: 12 additions & 12 deletions packages/beacon-state-transition/test/unit/util/slashing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import {assert} from "chai";
import {SLOTS_PER_EPOCH} from "@chainsafe/lodestar-params";
import {isSlashableAttestationData} from "../../../src/util";
import {randBetween} from "../../utils/misc";
import {generateAttestationDataBn} from "../../utils/attestation";
import {generateAttestationDataBigint} from "../../utils/attestation";

describe("isSlashableAttestationData", () => {
it("Attestation data with the same target epoch should return true", () => {
const epoch1 = randBetween(1, 1000);
const epoch2 = epoch1 + 1;
const a1 = generateAttestationDataBn(epoch1, epoch2);
const a2 = generateAttestationDataBn(epoch1 - 1, epoch2);
const a1 = generateAttestationDataBigint(epoch1, epoch2);
const a2 = generateAttestationDataBigint(epoch1 - 1, epoch2);
assert.isTrue(isSlashableAttestationData(a1, a2));
});

Expand All @@ -19,8 +19,8 @@ describe("isSlashableAttestationData", () => {
const epoch2 = epoch1 + 1;
const epoch3 = epoch1 + 2;
const epoch4 = epoch1 + 3;
const a1 = generateAttestationDataBn(epoch1, epoch2);
const a2 = generateAttestationDataBn(epoch3, epoch4);
const a1 = generateAttestationDataBigint(epoch1, epoch2);
const a2 = generateAttestationDataBigint(epoch3, epoch4);
assert.isFalse(isSlashableAttestationData(a1, a2));
});

Expand All @@ -32,14 +32,14 @@ describe("isSlashableAttestationData", () => {
const targetEpoch1 = randBetween(1, 1000);
const targetEpoch2 = targetEpoch1 - 1;

const a1 = generateAttestationDataBn(sourceEpoch1, targetEpoch1);
const a2Hi = generateAttestationDataBn(sourceEpoch2Hi, targetEpoch2);
const a1 = generateAttestationDataBigint(sourceEpoch1, targetEpoch1);
const a2Hi = generateAttestationDataBigint(sourceEpoch2Hi, targetEpoch2);

assert.isFalse(isSlashableAttestationData(a1, a2Hi));

// Second attestation has a smaller source epoch.
const sourceEpoch2Lo = sourceEpoch1 - 1;
const a2Lo = generateAttestationDataBn(sourceEpoch2Lo, targetEpoch2);
const a2Lo = generateAttestationDataBigint(sourceEpoch2Lo, targetEpoch2);
assert.isFalse(isSlashableAttestationData(a1, a2Lo));
});

Expand All @@ -55,16 +55,16 @@ describe("isSlashableAttestationData", () => {
// First slot in the epoch
let targetSlot2 = (targetEpoch - 1) * SLOTS_PER_EPOCH;

let a1 = generateAttestationDataBn(targetSlot1, sourceEpoch1);
let a2 = generateAttestationDataBn(targetSlot2, sourceEpoch2);
let a1 = generateAttestationDataBigint(targetSlot1, sourceEpoch1);
let a2 = generateAttestationDataBigint(targetSlot2, sourceEpoch2);

assert.isFalse(isSlashableAttestationData(a1, a2));

// Second attestation has a greater target epoch.
targetSlot1 = targetEpoch * SLOTS_PER_EPOCH;
targetSlot2 = (targetEpoch + 1) * SLOTS_PER_EPOCH;
a1 = generateAttestationDataBn(targetSlot1, sourceEpoch1);
a2 = generateAttestationDataBn(targetSlot2, sourceEpoch2);
a1 = generateAttestationDataBigint(targetSlot1, sourceEpoch1);
a2 = generateAttestationDataBigint(targetSlot2, sourceEpoch2);
assert.isFalse(isSlashableAttestationData(a1, a2));
});
});
2 changes: 1 addition & 1 deletion packages/beacon-state-transition/test/utils/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export function generateAttestationData(sourceEpoch: Epoch, targetEpoch: Epoch):
};
}

export function generateAttestationDataBn(sourceEpoch: Epoch, targetEpoch: Epoch): phase0.AttestationDataBn {
export function generateAttestationDataBigint(sourceEpoch: Epoch, targetEpoch: Epoch): phase0.AttestationDataBigint {
return {
slot: BigInt(0),
index: BigInt(0),
Expand Down
6 changes: 3 additions & 3 deletions packages/lodestar/test/unit/api/impl/beacon/pool/pool.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {expect} from "chai";
import sinon from "sinon";
import {generateAttestationDataBn} from "@chainsafe/lodestar-beacon-state-transition/test/utils/attestation";
import {generateAttestationDataBigint} from "@chainsafe/lodestar-beacon-state-transition/test/utils/attestation";
import {getBeaconPoolApi} from "../../../../../../src/api/impl/beacon/pool";
import {Network} from "../../../../../../src/network/network";
import {
Expand Down Expand Up @@ -88,12 +88,12 @@ describe("beacon pool api impl", function () {
const atterterSlashing: phase0.AttesterSlashing = {
attestation1: {
attestingIndices: [0],
data: generateAttestationDataBn(0, 1),
data: generateAttestationDataBigint(0, 1),
signature: Buffer.alloc(96),
},
attestation2: {
attestingIndices: [0],
data: generateAttestationDataBn(0, 1),
data: generateAttestationDataBigint(0, 1),
signature: Buffer.alloc(96),
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ describe("GossipMessageValidator", () => {
});

it("should return valid attester slashing", async () => {
const attestationData = ssz.phase0.AttestationDataBn.defaultValue();
const attestationData = ssz.phase0.AttestationDataBigint.defaultValue();
const attesterSlashing: phase0.AttesterSlashing = {
attestation1: {
data: attestationData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ describe("validate proposer slashing", () => {
});

it("should return valid proposer slashing", async () => {
const signedHeader1 = ssz.phase0.SignedBeaconBlockHeaderBn.defaultValue();
const signedHeader2 = ssz.phase0.SignedBeaconBlockHeaderBn.defaultValue();
const signedHeader1 = ssz.phase0.SignedBeaconBlockHeaderBigint.defaultValue();
const signedHeader2 = ssz.phase0.SignedBeaconBlockHeaderBigint.defaultValue();
// Make it different, so slashable
signedHeader2.message.stateRoot = Buffer.alloc(32, 1);

Expand Down
2 changes: 1 addition & 1 deletion packages/lodestar/test/utils/block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export function generateEmptySignedBlockHeader(): phase0.SignedBeaconBlockHeader
};
}

export function generateSignedBlockHeaderBn(): phase0.SignedBeaconBlockHeaderBn {
export function generateSignedBlockHeaderBn(): phase0.SignedBeaconBlockHeaderBigint {
return {
message: {
slot: BigInt(0),
Expand Down
Loading