set internal offset to 0 if no start time is provided. resolves discr…

…epency between start time and the value returned from reference clock causing non-zero offset. fix: #3271
trufflesuite · Jul 20, 2022 · 8d56eae · 8d56eae
1 parent 9df8402
commit 8d56eae
Show file tree

Hide file tree

Showing 8 changed files with 62 additions and 46 deletions.
diff --git a/src/chains/ethereum/block/tests/index.test.ts b/src/chains/ethereum/block/tests/index.test.ts
@@ -56,7 +56,7 @@ describe("@ganache/ethereum-block", async () => {
       blockchain = new Blockchain(
         options,
         fromAddress,
-        new ClockBasedBlockTime(new Date(), () => new Date())
+        new ClockBasedBlockTime(Date.now, undefined)
       );
       await blockchain.initialize(wallet.initialAccounts);
       // to verify our calculations for the block's baseFeePerGas,

diff --git a/src/chains/ethereum/ethereum/src/block-time.ts b/src/chains/ethereum/ethereum/src/block-time.ts
@@ -5,13 +5,13 @@ export interface BlockTime {
    * @returns number - The "offset" for the BlockTime instance in milliseconds.
    */
   setTime(timestamp: number | Date): number;
-  
+
   /**
    * Get the current offset of the BlockTime instance. This is the value that would be added to the system time in order to result in the desired timestamp.
    * @returns number - The "offset" for the BlockTime instance in milliseconds.
    */
   getOffset(): number;
-  
+
   /**
    * Set the current offset of the BlockTime instance. This is the value that would be added to the system time in order to result in the desired timestamp.
    * @param {number} offset - The "offset" for the BlockTime instance in milliseconds.
@@ -28,22 +28,25 @@ export interface BlockTime {
 
 export class ClockBasedBlockTime implements BlockTime {
   #getReferenceClockTime: () => number | Date;
-  #timeOffset: number;
+  #timeOffset: number = 0;
 
   private static validateTimestamp(timestamp: number | Date) {
     if (timestamp < 0) {
       throw new Error(`Invalid timestamp: ${timestamp}. Value must be positive.`);
     }
   }
 
-  constructor(startTime: number | Date, getReferenceClockTime: () => number | Date) {
-    ClockBasedBlockTime.validateTimestamp(startTime);
+  constructor(getReferenceClockTime: () => number | Date, startTime: number | Date | undefined) {
     this.#getReferenceClockTime = getReferenceClockTime;
-    this.setTime(startTime);
+
+    if (startTime !== undefined) {
+      ClockBasedBlockTime.validateTimestamp(startTime);
+      this.setTime(startTime);
+    }
   }
 
   getOffset(): number {
-    return this.#timeOffset;                              
+    return this.#timeOffset;
   }
 
   setOffset(offset: number) {
@@ -77,15 +80,15 @@ export class ClockBasedBlockTime implements BlockTime {
 /*
   A BlockTime implementation that increments it's reference time by the duration specified by incrementMilliseconds,
   every time createBlockTimestampInSeconds() is called. The timestamp returned will be impacted by the timestamp offset,
-  so can be moved forward and backwards by calling .putOffset() independent of the start time, and increments caused 
-  by calls to createBlockTimestampInSeconds()
+  so can be moved forward and backwards by calling .putOffset() independent of the start time, and will increment when
+  createBlockTimestampInSeconds() is called.
 */
 export class IncrementBasedBlockTime extends ClockBasedBlockTime {
   readonly #tickReferenceClock;
 
   constructor(startTime: number | Date, incrementMilliseconds: number) {
     let referenceTime = +startTime;
-    super(startTime, () => referenceTime)
+    super(() => referenceTime, startTime)
     this.#tickReferenceClock = () => referenceTime += incrementMilliseconds;
   }
 

diff --git a/src/chains/ethereum/ethereum/src/provider.ts b/src/chains/ethereum/ethereum/src/provider.ts
@@ -163,12 +163,13 @@ export class EthereumProvider
       providerOptions.fork.provider ||
       providerOptions.fork.network;
 
-    const startTime = this.#options.chain.time || new Date();
+    const startTime = this.#options.chain.time;
+    // if startTime is undefined, ClockBasedBlockTime will determine 0 offset from the reference time
     const blockTime =
       this.#options.miner.timestampIncrement === "clock"
-        ? new ClockBasedBlockTime(startTime, () => new Date())
+        ? new ClockBasedBlockTime(Date.now, startTime)
         : new IncrementBasedBlockTime(
-            startTime,
+            startTime || Date.now(),
             this.#options.miner.timestampIncrement.toNumber() * 1000
           );
 

diff --git a/src/chains/ethereum/ethereum/tests/api/debug/debug.test.ts b/src/chains/ethereum/ethereum/tests/api/debug/debug.test.ts
@@ -203,7 +203,7 @@ describe("api", () => {
         // using berlin here because we need this test to cost 0 gas
         EthereumOptionsConfig.normalize({ chain: { hardfork: "berlin" } }),
         address,
-        new ClockBasedBlockTime(new Date(), () => new Date())
+        new ClockBasedBlockTime(Date.now, undefined)
       );
 
       await blockchain.initialize(initialAccounts);

diff --git a/src/chains/ethereum/ethereum/tests/api/eth/call.test.ts b/src/chains/ethereum/ethereum/tests/api/eth/call.test.ts
@@ -868,7 +868,7 @@ describe("api", () => {
           blockchain = new Blockchain(
             options,
             new Address(wallet.addresses[0]),
-            new ClockBasedBlockTime(new Date(), () => new Date())
+            new ClockBasedBlockTime(Date.now, undefined)
           );
           await blockchain.initialize(wallet.initialAccounts);
 

diff --git a/src/chains/ethereum/ethereum/tests/block-time.test.ts b/src/chains/ethereum/ethereum/tests/block-time.test.ts
@@ -34,7 +34,7 @@ describe("block-time", () => {
     describe("constructor", () => {
       it("should throw with negative timestamp", () => {
         assert.throws(
-          () => new ClockBasedBlockTime(-100, () => 0),
+          () => new ClockBasedBlockTime(() => 0, -100),
           new Error(`Invalid timestamp: -100. Value must be positive.`)
         );
       });
@@ -44,8 +44,8 @@ describe("block-time", () => {
       it("should get a positive offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            futureTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            futureTimestamp
           );
           const offset = blockTime.getOffset();
 
@@ -61,8 +61,8 @@ describe("block-time", () => {
       it("should get a negative offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            pastTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            pastTimestamp
           );
           const offset = blockTime.getOffset();
 
@@ -78,8 +78,20 @@ describe("block-time", () => {
       it("should get a neutral offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            referenceTimestamp
+          );
+          const offset = blockTime.getOffset();
+
+          assert.strictEqual(offset, 0, "Unexpected offset");
+        });
+      });
+
+      it("should get a neutral offset when no startTime is provided", () => {
+        [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
+          const blockTime = new ClockBasedBlockTime(
+            () => referenceTimestamp,
+            undefined
           );
           const offset = blockTime.getOffset();
 
@@ -92,8 +104,8 @@ describe("block-time", () => {
       it("should set a positive offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
           blockTime.setOffset(duration);
           const offset = blockTime.getOffset();
@@ -105,8 +117,8 @@ describe("block-time", () => {
       it("should set a negative offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
           blockTime.setOffset(-duration);
           const offset = blockTime.getOffset();
@@ -118,8 +130,8 @@ describe("block-time", () => {
       it("should set a netural offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            pastTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            pastTimestamp
           );
           blockTime.setOffset(0);
           const offset = blockTime.getOffset();
@@ -131,8 +143,8 @@ describe("block-time", () => {
       it("should throw when resulting timestamp is negative", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
 
           assert.throws(() => blockTime.setOffset(-referenceTimestamp - 1));
@@ -144,8 +156,8 @@ describe("block-time", () => {
       it("should return a positive offset when setting a time in the future", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
           const offset = blockTime.setTime(futureTimestamp);
 
@@ -160,8 +172,8 @@ describe("block-time", () => {
       it("should return a negative offset when setting a time in the past", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
           const offset = blockTime.setTime(pastTimestamp);
 
@@ -174,7 +186,7 @@ describe("block-time", () => {
       });
 
       it("should throw with negative timestamp", () => {
-        const blockTime = new ClockBasedBlockTime(midTimestamp, () => midTimestamp);
+        const blockTime = new ClockBasedBlockTime(() => midTimestamp, midTimestamp);
 
         assert.throws(
           () => blockTime.setTime(-100),
@@ -187,7 +199,7 @@ describe("block-time", () => {
       it("should create a sequence of incrementing block timestamps", () => {
         [midTimestamp, +midTimestamp].forEach(startTime => {
           const clock = createTickingReferenceClock(startTime);
-          const blockTime = new ClockBasedBlockTime(startTime, clock);
+          const blockTime = new ClockBasedBlockTime(clock, undefined);
 
           for (let i = 0; i < 10; i++) {
             const expectedTimestampSeconds = Math.floor(+clock() / 1000);
@@ -204,7 +216,7 @@ describe("block-time", () => {
       it("should create a sequence of incrementing blockTimestamps with a positive offset", () => {
         [midTimestamp, +midTimestamp].forEach(startTime => {
           const clock = createTickingReferenceClock(startTime);
-          const blockTime = new ClockBasedBlockTime(startTime, clock);
+          const blockTime = new ClockBasedBlockTime(clock, undefined);
           blockTime.setOffset(duration);
 
           for (let i = 0; i < 10; i++) {
@@ -222,7 +234,7 @@ describe("block-time", () => {
       it("should create a sequence of incrementing blockTimestamps with a negative offset", () => {
         [midTimestamp, +midTimestamp].forEach(startTime => {
           const clock = createTickingReferenceClock(startTime);
-          const blockTime = new ClockBasedBlockTime(startTime, clock);
+          const blockTime = new ClockBasedBlockTime(clock, undefined);
           blockTime.setOffset(-duration);
 
           for (let i = 0; i < 10; i++) {
@@ -242,8 +254,8 @@ describe("block-time", () => {
       it("should create a blockTimestamp equal to the specified timestamp with a positive offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
 
           const blockTimestamp = blockTime.createBlockTimestampInSeconds(
@@ -268,8 +280,8 @@ describe("block-time", () => {
       it("should create a blockTimestamp equal to the specified timestamp with a negative offset", () => {
         [midTimestamp, +midTimestamp].forEach(referenceTimestamp => {
           const blockTime = new ClockBasedBlockTime(
-            referenceTimestamp,
-            () => referenceTimestamp
+            () => referenceTimestamp,
+            undefined
           );
 
           const blockTimestamp = blockTime.createBlockTimestampInSeconds(

diff --git a/src/chains/ethereum/ethereum/tests/blockchain.test.ts b/src/chains/ethereum/ethereum/tests/blockchain.test.ts
@@ -33,7 +33,7 @@ describe("blockchain", async () => {
       const blockchain = new Blockchain(
         options,
         initialAccounts[0].address,
-        new ClockBasedBlockTime(blockTime, () => new Date())
+        new ClockBasedBlockTime(Date.now, blockTime)
       );
       await blockchain.initialize(wallet.initialAccounts);
       const common = blockchain.common;

diff --git a/src/chains/ethereum/ethereum/tests/miner/miner.test.ts b/src/chains/ethereum/ethereum/tests/miner/miner.test.ts
@@ -49,15 +49,15 @@ describe("miner", async () => {
       lowGasLimitBlockchain = new Blockchain(
         options,
         fromAddress,
-        new ClockBasedBlockTime(new Date(), () => new Date())
+        new ClockBasedBlockTime(Date.now, undefined)
       );
       await lowGasLimitBlockchain.initialize(wallet.initialAccounts);
       optionsJson.miner.blockGasLimit = "0xB749E0"; // 12012000
       const highGasOptions = EthereumOptionsConfig.normalize(optionsJson);
       highGasLimitBlockchain = new Blockchain(
         highGasOptions,
         fromAddress,
-        new ClockBasedBlockTime(new Date(), () => new Date())
+        new ClockBasedBlockTime(Date.now, undefined)
       );
       await highGasLimitBlockchain.initialize(wallet.initialAccounts);