fix: fixing incremental token locks#119
Conversation
ryle-ai
left a comment
There was a problem hiding this comment.
Small and focused fix -- the current develop branch is missing the incrementalDelegatedStakingStartingOrdinal parameter in GlobalSnapshotContextFunctions.make() call (in TessellationServices.scala) and in the SharedConfig constructor (in Configuration.scala). This PR threads it through from the tessellation SharedConfigReader, which aligns with how SharedServices does it in tessellation proper (modules/node-shared/.../SharedServices.scala:182).
One note -- this has merge conflicts currently (status: CONFLICTING). The develop branch has moved forward with things like the clickHouseConfig and mptSnapshotInfoPath fields in SharedConfig, and the PR's Configuration.scala is based on an older shape. Should be a straightforward rebase but needs to happen before merge.
Also bumps the tessellation SNAPSHOT dep from d69174b96 to 5a4c2be69 -- I'm guessing this is the build that includes the incrementalDelegatedStakingStartingOrdinal parameter in GlobalSnapshotContextFunctions.make. Makes sense.
🤖 ryle-ai automated review
ryle-ai
left a comment
There was a problem hiding this comment.
The code changes themselves are the right shape — threading incrementalDelegatedStakingStartingOrdinal through SharedConfigReader into both SharedConfig and GlobalSnapshotContextFunctions.make is exactly what's needed. But the PR is missing the tessellation dependency bump that makes this compile.
The current dep in project/Dependencies.scala is 3.2.1-rc.2-179-a2e66401f-SNAPSHOT. The develop branch currently calls GlobalSnapshotContextFunctions.make with 6 arguments (through mptStore as the last). Adding a 7th argument (incrementalDelegatedStakingStartingOrdinal) without changing the dep means the call won't match any overload in the published artifact — this won't compile.
The tessellation field incrementalDelegatedStakingStartingOrdinal was introduced in commit c712ee979 (Jan 30 2026). If the dep hasn't been bumped past that commit, neither SharedConfigReader.incrementalDelegatedStakingStartingOrdinal nor the 7th parameter of GlobalSnapshotContextFunctions.make will exist in the resolved artifact.
Separate concern: this exact change was applied directly to develop as commit c776b11 and then reverted in b263dfb ("fix: manually revert to 3.5.12"). Before re-merging, it would help to know what the revert was about — was it the dep version specifically, or something else? If the dep issue was the root cause, the fix is a dep bump; if there was a correctness issue, that should be addressed here too.
| tessellation3Migration, | ||
| configuration.fieldsAddedOrdinals.setSumFix.getOrElse(env, SnapshotOrdinal.MinValue), | ||
| mptStore | ||
| mptStore, |
There was a problem hiding this comment.
GlobalSnapshotContextFunctions.make currently accepts 6 positional arguments in the published dep (a2e66401f). Adding a 7th here without bumping project/Dependencies.scala to a tessellation version that includes this parameter will cause a compile error. The dep needs to point to a build at or after tessellation commit c712ee979.
No description provided.