From fd29768c0a3dcf6c9dfe5860b41b46a0990b437a Mon Sep 17 00:00:00 2001
From: Nikos Baxevanis <nikos.baxevanis@gmail.com>
Date: Mon, 25 Mar 2024 23:41:11 +0100
Subject: [PATCH] refactor(pox-4): Dynamic `unlockBurnHt` and cleaner state

- Moved `network` from `Stub` (Model) to `Real`, streamlining state management.
- `unlockBurnHt` now dynamically calculated, passed via constructor, ensuring
  alignment with generated `period`.
- Utilized `fast-check`'s `chain` for dynamic `unlockBurnHt` calculation,
  improving test accuracy.
- Enforces discipline in constructor use and `check` method validation,
  highlighting the importance of explicit state and parameter handling for
  reliable testing.
---
 .../tests/pox-4/pox-4.stateful-prop.test.ts   |  1 -
 .../tests/pox-4/pox_CommandModel.ts           |  1 -
 .../tests/pox-4/pox_Commands.ts               | 22 ++++++++++---------
 .../pox-4/pox_DelegateStackStxCommand.ts      |  8 ++++---
 4 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts b/contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
index bd01ccce7c7..4331f434a2a 100644
--- a/contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
+++ b/contrib/core-contract-tests/tests/pox-4/pox-4.stateful-prop.test.ts
@@ -28,7 +28,6 @@ describe("PoX-4 invariant tests", () => {
     const model: Stub = {
       stackingMinimum: 0,
       wallets: new Map<StxAddress, Wallet>(),
-      network: sut.network
     };
 
     const wallets = [
diff --git a/contrib/core-contract-tests/tests/pox-4/pox_CommandModel.ts b/contrib/core-contract-tests/tests/pox-4/pox_CommandModel.ts
index 73ed4f9588f..ad5b3dd2fa3 100644
--- a/contrib/core-contract-tests/tests/pox-4/pox_CommandModel.ts
+++ b/contrib/core-contract-tests/tests/pox-4/pox_CommandModel.ts
@@ -9,7 +9,6 @@ export type StxAddress = string;
 export type Stub = {
   stackingMinimum: number;
   wallets: Map<StxAddress, Wallet>;
-  network: Simnet;
 };
 
 export type Real = {
diff --git a/contrib/core-contract-tests/tests/pox-4/pox_Commands.ts b/contrib/core-contract-tests/tests/pox-4/pox_Commands.ts
index 4ab67a63f9a..859316451ca 100644
--- a/contrib/core-contract-tests/tests/pox-4/pox_Commands.ts
+++ b/contrib/core-contract-tests/tests/pox-4/pox_Commands.ts
@@ -90,21 +90,23 @@ export function PoxCommands(
       }),
       period: fc.integer({ min: 1, max: 12 }),
       amount: fc.bigInt({ min:0n, max: 100_000_000_000_000n }),
-    }).map((
-      r: {
-        operator: Wallet;
-        stacker: Wallet;
-        startBurnHt: number;
-        period: number;
-        amount: bigint;
-      },
-    ) =>
+    }).chain((r) =>
+      fc.record({
+        unlockBurnHt: fc.constant(
+          currentCycleFirstBlock(network) + 1050 * (r.period + 1),
+        ),
+      }).map((unlockBurnHtRecord) => ({
+        ...r,
+        ...unlockBurnHtRecord,
+      }))
+    ).map((r) =>
       new DelegateStackStxCommand(
         r.operator,
         r.stacker,
         r.startBurnHt,
         r.period,
-        r.amount
+        r.amount,
+        r.unlockBurnHt,
       )
     ),
     // AllowContractCallerCommand
diff --git a/contrib/core-contract-tests/tests/pox-4/pox_DelegateStackStxCommand.ts b/contrib/core-contract-tests/tests/pox-4/pox_DelegateStackStxCommand.ts
index 4d6aafd0957..f23b4817a5c 100644
--- a/contrib/core-contract-tests/tests/pox-4/pox_DelegateStackStxCommand.ts
+++ b/contrib/core-contract-tests/tests/pox-4/pox_DelegateStackStxCommand.ts
@@ -2,7 +2,6 @@ import { PoxCommand, Real, Stub, Wallet } from "./pox_CommandModel.ts";
 import { poxAddressToTuple } from "@stacks/stacking";
 import { assert, expect } from "vitest";
 import { Cl, ClarityType, isClarityType } from "@stacks/transactions";
-import { currentCycleFirstBlock } from "./pox_Commands.ts";
 
 /**
  * The `DelegateStackStxCommand` locks STX for stacking within PoX-4 on behalf of a delegator.
@@ -29,6 +28,7 @@ export class DelegateStackStxCommand implements PoxCommand {
   readonly startBurnHt: number;
   readonly period: number;
   readonly amountUstx: bigint;
+  readonly unlockBurnHt: number;
 
   /**
    * Constructs a `DelegateStackStxCommand` to lock uSTX as a Pool Operator
@@ -40,6 +40,7 @@ export class DelegateStackStxCommand implements PoxCommand {
    * @param period - Number of reward cycles to lock uSTX.
    * @param amountUstx - The uSTX amount stacked by the Operator on behalf
    *                     of the Stacker
+   * @param unlockBurnHt - The burn height at which the uSTX is unlocked.
    */
   constructor(
     operator: Wallet,
@@ -47,12 +48,14 @@ export class DelegateStackStxCommand implements PoxCommand {
     startBurnHt: number,
     period: number,
     amountUstx: bigint,
+    unlockBurnHt: number,
   ) {
     this.operator = operator;
     this.stacker = stacker;
     this.startBurnHt = startBurnHt;
     this.period = period;
     this.amountUstx = amountUstx;
+    this.unlockBurnHt = unlockBurnHt;
   }
 
   check(model: Readonly<Stub>): boolean {
@@ -73,7 +76,6 @@ export class DelegateStackStxCommand implements PoxCommand {
 
     const operatorWallet = model.wallets.get(this.operator.stxAddress)!;
     const stackerWallet = model.wallets.get(this.stacker.stxAddress)!;
-    const unlockBurnHt = currentCycleFirstBlock(model.network) + 1050 * (this.period + 1)
 
     return (
       model.stackingMinimum > 0 &&
@@ -83,7 +85,7 @@ export class DelegateStackStxCommand implements PoxCommand {
       Number(this.amountUstx) <= stackerWallet.ustxBalance &&
       Number(this.amountUstx) >= model.stackingMinimum &&
       operatorWallet.hasPoolMembers.includes(stackerWallet.stxAddress) &&
-      unlockBurnHt <= stackerWallet.delegatedUntilBurnHt
+      this.unlockBurnHt <= stackerWallet.delegatedUntilBurnHt
     );
   }