Clarify #3977 with unbounded uint issue (#4018)

ChainSafe · May 13, 2022 · d56ff4f · d56ff4f
1 parent d7acdcf
commit d56ff4f
Show file tree

Hide file tree

Showing 17 changed files with 101 additions and 58 deletions.
diff --git a/packages/beacon-state-transition/src/allForks/block/isValidIndexedAttestation.ts b/packages/beacon-state-transition/src/allForks/block/isValidIndexedAttestation.ts
@@ -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.
@@ -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;
   }

diff --git a/packages/beacon-state-transition/src/allForks/block/processAttesterSlashing.ts b/packages/beacon-state-transition/src/allForks/block/processAttesterSlashing.ts
@@ -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
@@ -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`);
     }
   }

diff --git a/packages/beacon-state-transition/src/allForks/block/processProposerSlashing.ts b/packages/beacon-state-transition/src/allForks/block/processProposerSlashing.ts
@@ -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");
   }
 

diff --git a/packages/beacon-state-transition/src/allForks/signatureSets/attesterSlashings.ts b/packages/beacon-state-transition/src/allForks/signatureSets/attesterSlashings.ts
@@ -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));
@@ -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,
   };
 }
diff --git a/packages/beacon-state-transition/src/allForks/signatureSets/proposerSlashings.ts b/packages/beacon-state-transition/src/allForks/signatureSets/proposerSlashings.ts
@@ -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,
       };
     }

diff --git a/packages/beacon-state-transition/src/util/attestation.ts b/packages/beacon-state-transition/src/util/attestation.ts
@@ -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)
   );

diff --git a/packages/beacon-state-transition/test/perf/phase0/block/util.ts b/packages/beacon-state-transition/test/perf/phase0/block/util.ts
@@ -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,

diff --git a/packages/beacon-state-transition/test/unit/signatureSets/signatureSets.test.ts b/packages/beacon-state-transition/test/unit/signatureSets/signatureSets.test.ts
@@ -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),
@@ -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,
   };
 }
@@ -136,7 +136,7 @@ function getAttestationData(): phase0.AttestationData {
   };
 }
 
-function getAttestationDataBn(): phase0.AttestationDataBn {
+function getAttestationDataBigint(): phase0.AttestationDataBigint {
   return {
     slot: BigInt(0),
     index: BigInt(0),

diff --git a/packages/beacon-state-transition/test/unit/util/slashing.test.ts b/packages/beacon-state-transition/test/unit/util/slashing.test.ts
@@ -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));
   });
 
@@ -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));
   });
 
@@ -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));
   });
 
@@ -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));
   });
 });
diff --git a/packages/beacon-state-transition/test/utils/attestation.ts b/packages/beacon-state-transition/test/utils/attestation.ts
@@ -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),

diff --git a/packages/lodestar/test/unit/api/impl/beacon/pool/pool.test.ts b/packages/lodestar/test/unit/api/impl/beacon/pool/pool.test.ts
@@ -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 {
@@ -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),
       },
     };

diff --git a/packages/lodestar/test/unit/chain/validation/attesterSlashing.test.ts b/packages/lodestar/test/unit/chain/validation/attesterSlashing.test.ts
@@ -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,

diff --git a/packages/lodestar/test/unit/chain/validation/proposerSlashing.test.ts b/packages/lodestar/test/unit/chain/validation/proposerSlashing.test.ts
@@ -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);
 

diff --git a/packages/lodestar/test/utils/block.ts b/packages/lodestar/test/utils/block.ts
@@ -75,7 +75,7 @@ export function generateEmptySignedBlockHeader(): phase0.SignedBeaconBlockHeader
   };
 }
 
-export function generateSignedBlockHeaderBn(): phase0.SignedBeaconBlockHeaderBn {
+export function generateSignedBlockHeaderBn(): phase0.SignedBeaconBlockHeaderBigint {
   return {
     message: {
       slot: BigInt(0),