Inconsistent block height output

[whoabuddy]

Describe the bug

We have a few contracts that create proposals and use at-block to get snapshot balances.

For one contract in particular, it tracks the last block height a proposal was created at:

(define-public (propose-action (action <action-trait>) (parameters (buff 2048)))
  (let
    (
      (actionContract (contract-of action))
      (newId (+ (var-get proposalCount) u1))
      (liquidTokens (try! (get-liquid-supply block-height)))
      (createdAt block-height)
      (startBlock (+ burn-block-height VOTING_DELAY))
      (endBlock (+ startBlock VOTING_PERIOD))
      (senderBalance (unwrap! (contract-call? .aibtc-token get-balance tx-sender) ERR_FETCHING_TOKEN_DATA))
      (validAction (is-action-valid action))
    )
    ;; liquidTokens is greater than zero
    (asserts! (> liquidTokens u0) ERR_FETCHING_TOKEN_DATA)
    ;; verify this extension and action contract are active in dao
    (asserts! validAction ERR_INVALID_ACTION)
    ;; at least one stx block has passed since last proposal
    (asserts! (> createdAt (var-get lastProposalCreated)) ERR_ALREADY_PROPOSAL_AT_BLOCK)
    ;; caller has the required balance
    (asserts! (> senderBalance u0) ERR_INSUFFICIENT_BALANCE)
    ;; print proposal creation event
    (print {
      notification: "propose-action",
      payload: {
        proposalId: newId,
        action: actionContract,
        parameters: parameters,
        caller: contract-caller,
        creator: tx-sender,
        createdAt: createdAt,
        startBlock: startBlock,
        endBlock: endBlock,
        liquidTokens: liquidTokens,
      }
    })
    ;; create the proposal
    (asserts! (map-insert Proposals newId {
      action: actionContract,
      parameters: parameters,
      caller: contract-caller,
      creator: tx-sender,
      createdAt: createdAt,
      startBlock: startBlock,
      endBlock: endBlock,
      liquidTokens: liquidTokens,
      votesFor: u0,
      votesAgainst: u0,
      concluded: false,
      metQuorum: false,
      metThreshold: false,
      passed: false,
      executed: false,
    }) ERR_SAVING_PROPOSAL)
    ;; set last proposal created block height
    (var-set lastProposalCreated createdAt)
    ;; increment proposal count
    (ok (var-set proposalCount newId))
  )
)

However in the tests we cannot get the block heights to line up, relevant testing code for get-last-proposal-created:
https://github.com/aibtcdev/aibtcdev-contracts/blob/ec3dcac45a5e251fe021b22ac0071abfa043d48e/tests/dao/extensions/aibtc-action-proposals-v2.test.ts#L1091-L1198

The test fails because the block height doesn't match, but after console logging it doesn't make sense to me why the numbers are off following the progression:

 FAIL  tests/dao/extensions/aibtc-action-proposals-v2.test.ts > read-only functions: aibtc-action-proposals-v2 > get-last-proposal-created(): succeeds and returns the block height of the last proposal
AssertionError: expected { type: 1, value: 15n } to strictly equal { type: 1, value: 21n }

And what we see in the console logs of the test (left them in since this one still fails):

epoch at start: 3.0
block height at start: 6
clarity version: Clarity 3
stacksBlockHeight: 6
block height after dao tokens call: 7
block height after mine empty blocks (10): 17
block height after construct dao: 18
block height after create proposal: 19 (we set it here)
from get last proposal created: 14
block height after mine empty block: 20
block height after create 2nd proposal: 21 (we set it here)
from get last proposal created 2: 15

To Reproduce
Steps to reproduce the behavior:

1. git clone [email protected]:aibtcdev/aibtcdev-contracts
2. git checkout feat/proposals-v2
3. npm i && npm run test

Expected behavior
I expected the block heights captured from simnet would match what's recorded in the contract for the public function call.

Environment (please complete the following information):

• OS (version): Linux Mint 20.3
• Clarinet version [e.g. 1.7.0] - clarinet 2.12.0

Additional context

We also tried switching all contracts to clarity_version 3 and using stacks-block-height with get-stacks-block-info? but this caused every attempt at getting the block hash in the contracts to fail. Needs more investigation and documentation isn't clear on when that goes into effect.

We also noted that the clarity_version 2 defaults above will deploy, so sticking with them for now unless we need to update. Is there any ETA on this?

• example of deployed contracts: https://explorer.hiro.so/address/SP2XCME6ED8RERGR9R7YDZW7CA6G3F113Y8JMVA46

[whoabuddy]

We were able to work this out for now by extracting the block height from the print event, but it would still be nice to have guidance on how to handle block height in relation to mineEmptyBlock() or function calls. Should we expect to call the global simnet.blockHeight before/after operations to get the correct height?

[hugocaillard]

Hey @whoabuddy
Thanks for the detailled issue.
Can you try with this vitest config:

export default defineConfig({
  test: {
    environment: "clarinet", // use vitest-environment-clarinet
    pool: "forks",
    poolOptions: {
      forks: { singleFork: true },
    },
    setupFiles: [
      vitestSetupFilePath,
      // custom setup files can be added here
    ],
    environmentOptions: {
      clarinet: {
        ...getClarinetVitestsArgv(),
        // add or override options
      },
    },
  },
});

The pool option was threads but I replaced it with forks, it has proven to be more stable with the siment handling.

And singleFork: true ensures that tests run in the same thread; using only one simnet instances (the deployment is cached) it should run the tests faster. Test isolation is still provided (each state with restart from a clean state)

More details in the docs: https://vitest.dev/config/#pooloptions.

[hugocaillard]

We also tried switching all contracts to clarity_version 3 and using stacks-block-height with get-stacks-block-info? but this caused every attempt at getting the block hash in the contracts to fail. Needs more investigation and documentation isn't clear on when that goes into effect.

This should work. I'd be interested in seeing a minimal reproducible example for that

[whoabuddy]

This is what I have now for the config, iirc we added singleThread: true because we were having some tests run more than once but it wasn't clear which flags to play with.

export default defineConfig({
  test: {
    environment: "clarinet", // use vitest-environment-clarinet
    singleThread: true,
    setupFiles: [
      vitestSetupFilePath,
      // custom setup files can be added here
    ],
    environmentOptions: {
      clarinet: {
        ...getClarinetVitestsArgv(),
        retryCount: 0,
        // add or override options
      },

    },
  },
});

Can you try with this vitest config:

Updating to the provided vitest config format runs a bit faster and I get less punycode warnings, so that's nice! I'm still seeing a 5 block difference between the value I expect and the transactions I'm currently working on as of this commit

blockheight console output:

current epoch 3.0
clarity version Clarity 3
starting block 6
after dao tokens receipt 7
after mine empty blocks 17
after construct dao 18
after mine empty blocks 450
after create proposal 451
compared to createdAt 451 446
after mine empty blocks 461
after create proposal 2 462
compared to createdAt2 462 456

This should work. I'd be interested in seeing a minimal reproducible example for that

I also pushed the next commit with an example of what happens when we update to clarity_version = 3 and use stacks-block-height and get-stacks-block-info?.

The code checks fine with Clarinet but when I run the tests there are several 1009 / 3009 errors which equate to ERR_RETRIEVING_START_BLOCK_HASH in the core and action proposal contracts. The only change are those two keywords so I'd expect it to work the same, and the tests should progress the block height when needed (e.g. for at-block calls). The fact it worked before and not after the change left me wondering what we did wrong.

[hugocaillard]

This is what I have now for the config, iirc we added singleThread: true because we were having some tests run more than once but it wasn't clear which flags to play with.

This was the Vitest v0.x config schema (test.singleThread: bool). If you are using vitest v1.x (and you should), it won't have any effect.
Hence why I sent you and updated config, you should stick to it.

The fact it worked before and not after the change left me wondering what we did wrong

It could be a bug in Clarinet.

---

I'll have to spend more time on it this week or next one.
But I suspect it will take me quite some time to dig into your repo and create a reproducible example in order to pin point the issue

[whoabuddy]

This was the Vitest v0.x config schema (test.singleThread: bool). If you are using vitest v1.x (and you should), it won't have any effect. Hence why I sent you and updated config, you should stick to it.

Great, happy to use that config instead! I've started a lot of different projects at different times it's really hard to tell when everything is "up to date" and/or configured correctly. Sometimes it's easier to copy an old project then scaffold a whole new one which complicates the issue.

(e.g. this repo is on @stacks/transactions 6.x and needs to migrate to 7.x, could be using vite 6 / vitest 3, but it's working well in its current state)

"dependencies": {
  "@hirosystems/clarinet-sdk": "^2.12.0",
  "@stacks/transactions": "^6.17.0",
  "chokidar-cli": "^3.0.0",
  "typescript": "^5.7.3",
  "vite": "^5.4.11",
  "vitest": "^1.6.0",
  "vitest-environment-clarinet": "^2.1.0"
}

I'll have to spend more time on it this week or next one.
But I suspect it will take me quite some time to dig into your repo and create a reproducible example in order to pin point the issue

Happy to help if any questions or if I have time I'll make a minimal example.

• in the contract: epoch = 3.0, clarity_version = 2, stores block-height in a map, getter for block height
• in the test: track simnet.blockHeight output alongside public function calls (should increase 1 with each), see if block height matches getter result

Another thing I've noticed is we do not explicitly initSimnet() but use a globally available variable, do we have to?

And we have some methods as "helpers" that perform a few actions, e.g. send 3 votes on a proposal then conclude it and return the receipt.

Would love to figure out the best practices here for this and other projects!

[hugocaillard]

The clarinet-sdk works with @stacks/transactions v6 and vitest v1, so this is good 👍
We started working on support for v7 and v3, but that will be clarinet-sdk v3.

Another thing I've noticed is we do not explicitly initSimnet() but use a globally available variable, do we have to?

Yes thats good. That's thanks to vitest-environment-clarinet, it takes care of the init, and then the session is initialised before each test (providing tests isolation).
Having this global simnet instance gives the ability to easily collect code coverage.

Would love to figure out the best practices here for this and other projects!

I think this repo looks good already.
If you find yourself calling the same helper again and again, you can put in in a beforeEach instead. (The beforeEach can be inside a describe to be scoped to it)

Happy to help if any questions or if I have time I'll make a minimal example.

Thanks 🙏
That would be awesome

[whoabuddy]

The clarinet-sdk works with @stacks/transactions v6 and vitest v1, so this is good 👍
We started working on support for v7 and v3, but that will be clarinet-sdk v3.

Excellent, good to know! What about using the init functions for simnet? So far I'm just using the global and noticed it did the cleanup before each run. That's definitely useful for unit tests, and in Deno we fought so much with beforeEach() and beforeAll() so it will be great to start working with those in mind again.

Thanks 🙏
That would be awesome

Part of the way there, scaffolded and still need to add some more test cases.
https://github.com/whoabuddy/clarinet-blockheight-test