Add initDevMode function with environment variable support#1133
Add initDevMode function with environment variable support#1133Copilot wants to merge 3 commits into
Conversation
…tically Co-authored-by: vicsn <24724627+vicsn@users.noreply.github.com>
|
Can't add you as a reviewer natively so: CC @14MR |
|
Maybe we could set it as environment variable? Then developers can run the same application code without need to handle it by themselves |
|
…tection Co-authored-by: vicsn <24724627+vicsn@users.noreply.github.com>
Added import { maybeInitDevMode, initThreadPool } from "@provablehq/sdk";
maybeInitDevMode(); // Auto-enables dev mode if ALEO_NETWORK=local
await initThreadPool(); |
|
I am thinking if we should use Only thing i am concerned about is if it will be ok to run with real RPC (testnet/mainnet) + running in dev mode. But i guess our testnet and mainnet are always on latest, so should be ok? @copilot wdyt? |
I think this is generally not a good idea. Consider that the consensus versions dictates how one creates and verifies transactions. If a new snarkVM and SDK release is out, but the latest consensus version will only be active X days from now, we still want to enable developers to deploy the new SDK so they don't incur any downtime once the hard fork occurs and people want to use new features. |
|
TODO::
|
They certainly do during executions and authorization building, for the SDK to be able to be used against a devnode, developers should be able to set it. |
iamalwaysuncomfortable
left a comment
There was a problem hiding this comment.
This is correct-ish. The wasm function works, but loading the configuration needs more thought than this provides.
The Process detection is a poor way to detect the node environment as any number of global objects named process might exist due to polyfill or other imports in the program's build chain.
The proper way to do this is within the node.ts file.
Envar detection is desirable in the node context. We should name it ConsensusHeights to be consistent with the Leo CLI and if the users have specified consensus heights, take those, or if the envar is specified but blank, just use the default heights.
| export function maybeInitDevMode(): number[] | undefined { | ||
| try { | ||
| if (typeof process !== 'undefined' && process.env?.ALEO_NETWORK === 'local') { | ||
| return initDevModeWasm(); | ||
| } | ||
| } catch { | ||
| // Ignore errors when process is not available (e.g., in browsers) | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
This process detection is inane. Node.ts already runs all node related code, so anything that is set by default can be set there.
| export function initDevMode(): number[] { | ||
| return initDevModeWasm(); | ||
| } |
There was a problem hiding this comment.
This extra wrapper doesn't add much, the wasm method itself is sufficient and can be exported directly.
|
This PR is a total mess at this point but I hope we can find common ground on the design.
In that case, how about the following idea to set the heights automatically:
@iamalwaysuncomfortable would this make sense, and where could be a logical place to query for the consensus heights? |
@vicsn on the surface it makes sense, but it has some unfortunate tradeoffs. In order of increasing severity:
While the above approach would likely work most of the time, it's quite probabilistic and subject to some known footguns. Those things make the approach just chaotic enough to cause some trouble for developers and we should aim for the deterministic approach that bases things on the heights existing in SnarkVM. The SDK IS SnarkVM compiled to My opinion is that developers should just call In I'll address this tomorrow fully. |
|
Aside from making a network request, is there any other way to automatically know if dev mode should be enabled or not? If not... then I agree with @iamalwaysuncomfortable that it should be something that is manually set by the user. And since it needs to be manually set by the user, I think the best option is to just put it into the code: import { GlobalSettings } from "@provablehq/sdk";
GlobalSettings.devMode = true;
// Rest of code...Are there any problems with this approach? |
I think we can anyhow start with this, and we can evaluate methods to automate setting it in the future. Can I nominate @14MR to bring this PR over the finish-line? To everyone who reads this: It is low priority so this PR will likely be untouched for a few weeks |
Motivation
Setting consensus version heights manually via
getOrInitConsensusVersionTestHeights("0,1,2,3,4,5,6,7,8,9,10,11")requires developers to know the exact number of consensus versions upfront—error-prone and tedious.This adds
initDevMode()which readsConsensusVersion::latest()and automatically sets heights[0..latest]. Additionally,maybeInitDevMode()provides environment variable detection for automatic dev mode initialization.Changes
wasm/src/utilities/mod.rs: Addedinit_dev_mode()function that queriesConsensusVersion::latest()and passes"0,1,2,...,n"to existingget_or_init_consensus_version_heightssdk/src/wasm.ts: Added TypeScript wrapperinitDevMode()with JSDoc documentation, plusmaybeInitDevMode()that checks forALEO_NETWORK=localenvironment variablesdk/src/browser.ts: ExportinitDevModeandmaybeInitDevModeTest Plan
cargo check --features mainnetandcargo check --features testnetgetOrInitConsensusVersionTestHeightsmaybeInitDevMode()safely handles browser environments whereprocess.envis unavailableRelated PRs
N/A
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.