circomkit icon indicating copy to clipboard operation
circomkit copied to clipboard

Add teardown to cli commands

Open ScreamingHawk opened this issue 1 year ago • 1 comments

Added the teardown script to all CLI commands. Solves #95

Note: While not all commands need tearing down, this pattern will keep the code maintainable.

ScreamingHawk avatar Oct 03 '24 01:10 ScreamingHawk

Can you do something like this in the program chain around line 223 instead so it's just in one spot?

program.hook('postAction', () => {
  // SnarkJS may attach curve_bn128 to global, but does not terminate it.
  // We have to do it manually (see https://github.com/iden3/snarkjs/issues/152)
  if (globalThis.curve_bn128) globalThis.curve_bn128.terminate();
});

I think the comment about the related snarkjs issue that accompanies the line is important.


Edit: it should actually probably just do the curve terminate in the main code where it's needed after invoking snarkjs, then we can be sure it's fixed because we can remove the teardown code from the tests. The only issue with that might be performing operations simultaneously using Promise.all

numtel avatar Oct 03 '24 06:10 numtel

hmmm yep indeed this may be the fix, thanks for bringing it up!

lemme see if Yargs has a nicer way to do this instead of repeating the code like this

EDIT: I meant Commander instead of Yargs :D

erhant avatar Oct 06 '24 16:10 erhant

alright, I used postAction hook of Commander to write terminate once, as @numtel also suggested

also, terminate turned out to be an async function (see https://github.com/iden3/ffjavascript/blob/master/src/bn128.js#L48) so I made it so as well

erhant avatar Oct 06 '24 16:10 erhant