hirosystems
Inconsistent block height output
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:
git clone [email protected]:aibtcdev/aibtcdev-contractsgit checkout feat/proposals-v2npm 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
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?
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.
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
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.
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
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.blockHeightoutput 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!
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
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
So we're still seeing this when we make certain changes to our tests, the block heights shift a bit and there's stlil ~5 block discrepancy between the set and expected values.
Sometimes when tests mess up I end up rm -rf deployments && clarinet check && npm run test and it fixes it, and we're still running occasionally seeing conflicts like those listed in #1579.
Maybe this issue is related to the work for #1579 and #1368?
@whoabuddy Clarinet 2.13.0 fixes a few bugs, included the one where you would see empty batches being added the deployment plan after runnning npm test.
But indeed #1368 hasn't been addressed yet so it's likely that you'll run into some issue when relying on both clarinet check and npm test. If that's an issue, you can only rely on npm test
@whoabuddy I just tried running the tests on the feat/proposals-v2 with the latest [email protected].
And they seem to pass.
Are you still experiencing inconsistent block height?
While I didn't directly tried to fix this bug, I recently revamped the clarity datastore quite a lot, fixing bug and weird behaviors along the way
I'll install the latest and test again then create an isolated test as an example. The issue was in two contracts that record their stacks and burn block height at deployment and return it later in a getter function - when I tried to match that up to the simnet.blockHeight() functions there was no easy way to say "deployed at bitcoin block 4".
I have it hardcoded right now and have to adjust it whenever the deployment plan changes. How can I better understand how blocks progress on simnet and what to expect? aibtc-action-proposals-v2 aibtc-core-proposals-v2