[DRAFT] Experimental: First-class citizenship refactoring

[BowTiedRadone]

This PR is meant to simplify the Rendezvous tests first class citizenship logic by passing the deployment plan responsibility down to clarinet-sdk. It is an experimental approach which aims to minimize the internal complexity and improve Rendezvous' overall reliability.

Closes #180 as a side effect of the refactoring.

This PR is still a draft.

[moodmosaic]

I think this is a great example of working with clarinet-sdk instead of around it.

Replacing manual contract orchestration with a simple temporary manifest reduces complexity (also, no more manual balance saving/restoration logic, no more manual contract deployment orchestration). 💯

Since this now relies on temp files, it could introduce issues like file permissions or cleanup failures. Good to see the cleanup is in a finally block, but this should still be tested thoroughly (see PoLA).

[BowTiedRadone]

@moodmosaic Now that I'm experimenting this approach, I'm thinking if it'd make sense to have a run directory which will contain the data generated from the user inputs, and could hold more data in the future (e.g. failed seeds). This would look similarly to echidna's output dir. The user could also specify the directory where this data should be stored.

[hugo-stacks]

Since this will be implemented in clarinet (stx-labs/clarinet#2022), my recommendation would be to pause in rendezvous for now

[BowTiedRadone]

@hugo-stacks I decided to try and merge this, as it fixes an existing issue. This also simplifies the codebase, preparing it for when stx-labs/clarinet#2022 is ready.

[moodmosaic]

The architectural simplification comes at what appears to be a high cost to me, but I think that's a conclusion to be made only after you go through all comments. Perhaps some of them are invalid or based on misunderstandings of the intent. Massive work!

[moodmosaic]

It might be worth considering doing some cleanup here for orphaned files, e.g.:

// Add startup cleanup for orphaned files.
function cleanupOrphanedTempFiles(manifestDir: string) {
  const tempManifestPattern = /^\.Clarinet\.toml\.\w+$/;
  // Clean up old temp files...
}

I mean, the finally block helps, but doesn't protect against: SIGKILL, power loss, system crash, etc.

[moodmosaic]

This is called twice (as per CTRL+F at least). Why?

Perhaps just

// Single initialization with validation.
const deploymentPlan = await validateAndGenerateDeploymentPlan(manifestPath);
const simnet = await initSimnetWithRendezvous(deploymentPlan, rendezvousContract);

can be enough?

[BowTiedRadone]

The first simnet initialization generates the deployment plan. The deployment plan (mainly) contains info regarding requirement contracts. Given the end goal of this refactoring (stx-labs/clarinet#2022), fuzzing requirement contracts will no longer be possible (since the user will only be able to add Rendezvous tests as conditionally deployed code in the target contract file), so it could make sense exploring a way to only initialize the session once. Thanks for the great comment!

[BowTiedRadone]

I don't think there's a publicly exposed simnet method to only generate the deployment plan, but @hugo-stacks can correct me if I'm wrong.

[moodmosaic]

I think this needs some thinking. Users won't know if cleanup failed, leading to disk bloat.

At the very least:

} catch (error) {
  radio.emit("logMessage", 
    `Warning: Failed to cleanup temporary directory: ${error.message}`
  );
}

[moodmosaic]

Why remove macos-latest? Blind spot for platform-specific issues.

[BowTiedRadone]

Since simnet is now initialized twice, running tests became resource-heavy. The macOS runners' limited resources and availability led to tests taking too long (eventually generating cancelled jobs). This reason plus the fact that this job only runs tests (not actually running Rendezvous against example contracts) convinced me to drop macOS from the tests OS matrix. Running examples still happens for ubuntu, windows, and macOS.

Do we really want to run the tests on both ubuntu and macOS, or are we fine with only the examples being run (at least that's what the user can use cross-platform)?

[moodmosaic]

This is repeated in multiple places, invoking rule of three:

function getCacheDir(manifest: any): string {
  return manifest.project?.cache_dir ?? "./.cache";
}

[moodmosaic]

Instead of temporary files, what if something like this could work?

export const issueFirstClassCitizenship = async (
  manifestDir: string,
  manifestPath: string,
  sutContractName: string,
  radio: EventEmitter
): Promise<Simnet> => {
  // Parse once.
  const manifest = parseManifest(manifestPath);
  const deploymentPlan = await generateDeploymentPlan(manifest);
  
  // Build rendezvous in memory.
  const rendezvousContract = buildRendezvousContract(
    deploymentPlan, 
    sutContractName
  );
  
  // Init simnet with custom contract source resolver.
  const simnet = await initSimnetWithCustomContracts(
    manifest,
    { [sutContractName]: rendezvousContract }
  );
  
  return simnet;
};

If this works, it avoids temporary files, double initialization, disk I/O, cleanup complexity.

[BowTiedRadone]

@hugo-stacks Any way to stream the manifest file instead of providing the file path?

[BowTiedRadone]

@moodmosaic Thanks for the solid review! It led me to what I think is an overall cleaner approach. I have updated the flow to use an isolated copy of the Clarinet project, tailored for the Rendezvous run. The same approach proved to be effective back when I addressed #173. In the future this could turn into a run output directory, containing regression seeds and other useful info (similar to how echidna does it). For now, there is no cleanup for the temporary isolated run directories (temp files are not reboot resistant), and failed runs can also bring value as the user can check what went wrong (if that's the case).

Next, the flow I plan to implement on top of the current approach is cleanup by default, overridable by a --keep-working-dir (example naming) boolean flag. However, a second opinion on the new approach would be highly appreciated before continuing. Please also keep in mind that this PR is a temporary preparation for stx-labs/clarinet#2022, and it fixes an issue encountered during real Rendezvous usage.