[Feature] No access to accounts in preSetup about clarinet

[MarvinJanssen]

Point is that people might not be aware of the order of operations and try to deploy a mock after calling mineBlock() by accident. That's why I like the readability of having a separate preSetup(). (Which we should arguably just call setup().) It also makes it easier to define a few setups that can be reused for different tests.

Changing mock behavior in the middle of a test? To me it screams for a separate test.

Indeed, which is why I wondered if there is a valid use-case at all.

If there would be a way to intercept contract-calls sent to a mock, pass them to JS that would calculate valid response that would be a game changer :)

Yep, I explored this but it leaves things to be desired. You'd need to do some interception and have it call back into a callback defined in test. Sounds tricky. It is why my MockContract sample generates all that is needed in JS, keeping Clarinet light. I think we can get away with setting a static return value from a mock call. I'm not sure if there are real use-cases for requiring calculation. (Do you have any examples?) The only thing we might want is a way to trigger a side-effect, like updating a map or data var in the mock contract to be used by a subsequent tx in the same test, or triggering a panic or event. But one can again wonder if setting static return values is enough. As any data to be returned from a public function can likewise be mocked.

[LNow]

@MarvinJanssen can you give an example of things you would like to do in preSetup()?

[MarvinJanssen]

Deploy mock contracts or send initialising calls to contracts. Some of these things could be alleviated with deploy scripts but anything that is statically coded into the contract has to be present before Clarinet does its chain setup. Say that you are working on a prediction market that uses open-oracle, you'd want to mock that oracle in test.

[LNow]

What if we would split setup_chain into two separate phases?

1. setup_accounts
2. setup_contracts

If it would be possible, we could start writing like tests like this:

Clarinet.test('Super important test', (chain: Chain, accounts: Accounts) => {
  let mockOracle = new MockContract("imaginary-oracle-v1", "SP000000000000000000002Q6VF78");
	mockOracle.publicFn("price",{
    inputs: [typeDefs.ascii(24), typeDefs.ascii(24), typeDefs.uint()],
    returns: types.ok(types.uint(2000000))
  });
  
  // we could also have a addSetupTx(tx: Tx) function
  chain.addMock(mockOracle);

  const wallet_1 = accounts.get("wallet_1")!;
  
  let block = chain.mineBlock([
    Tx.contractCall(`SP000000000000000000002Q6VF78.imaginary-oracle-v1`, "price",
    [
        types.ascii("usd"),
        types.ascii("stx"),
        types.uint(5)
    ], wallet_1.address),
  ]);

  console.info(block)
})

As long as we won't call mineBlock, mineEmptyBlock, mineEmptyBlockUntil, callReadOnlyFn, getAssetsMaps only accounts would be set up.
Once we call any of these functions, then all our contracts as well as mocks will be deployed and function we call will be executed.

I think this should give us a bit more pleasant so called developer experience.

Clarinet code would look then more or less like this:

export interface Accounts extends Map<string, Account> {}

export class Clarinet {
  public static test(name: string, fn: (chain:Chain, accounts: Accounts)=> void){  
    let chain = new Chain(name);
    let accounts = chain.accounts;

    const testDefinition: Deno.TestDefinition = {
      name: name,
      fn: async () => {
        await fn(chain, accounts);
      }
    }

    Deno.test(testDefinition);
  }
}

export class Chain {
  sessionId: number;
  accounts: Accounts = new Map();
  private isReady: boolean = false;
  private transactions: Array<Tx> = [];
  private name: string;

  constructor(name: string) {
    this.name = name;

    (Deno as any).core.ops();
    let result = (Deno as any).core.jsonOpSync("setup_account");

    this.sessionId = result["session_id"]

    for (let account of result["accounts"]) {
      this.accounts.set(account.name, account);
    }
  }

  private setUp() {
    let result = (Deno as any).core.jsonOpSync("setup_contracts", {
      name: this.name,
      transactions: this.transactions
    })
    // we reset accounts as their balances might be different after this phase
    this.accounts.clear();
    for (let account of result["accounts"]) {
      this.accounts.set(account.name, account);
    }

    this.isReady = true;
  }

  addMock(mock: MockContract):void {
    this.transactions.push(Tx.deployContract(mock));
  }

  mineBlock(transactions: Array<Tx>) {
    if(!this.isReady) {
      this.setUp();
    }

    let result = (Deno as any).core.jsonOpSync("mine_block", {
      sessionId: this.sessionId,
      transactions: transactions,
    });
    let block: Block = {
      height: result.block_height,
      receipts: result.receipts,
    };
    return block;
  }
....
....
}

[MarvinJanssen]

Interesting idea, although I wonder if this would be clear to people. I can see that people will try to deploy mocks after calling those functions.

If one can only deploy mocks during preSetup(), then there is a clear division. (Although I wonder if there would be a valid reason for being able to change the output / side-effects of a mock during the test() phase? If so, then then these could also be passed to test(), see #40. )

[LNow]

The thing is that if your contract somehow depends on a mock, you can't deploy it after. It must be deployed before you can any function in you contract. Otherwise you won't be able to deploy it.

Deploying mock is nothing else but part of Arrange phase of your test.
With current test function signature you have something like this:

Clarinet.test({
  name: 'name of your test',
  preSetup(): Tx[] {
    //ARRANGE 
    // some setup transactions
  },
  async fn(chain: Chain, accounts: Map<string, Account>) {
    //ARRANGE - continuation
    
    //ACT

   //ASSERT
  }
});

And I think this one is a bit cleaner (only one arrange section):

Clarinet.test('name of your test', async (chain: Chain, accounts: Accounts) => {
   // ARRANGE

   // ACT

   // ASSERT
});

Changing mock behavior in the middle of a test? To me it screams for a separate test.
If there would be a way to intercept contract-calls sent to a mock, pass them to JS that would calculate valid response that would be a game changer :)
Baby steps - we'll get there.

[lgalabru]

I'm trying to prioritize, is this feature request solving actual pain points? cc @MarvinJanssen @LNow

[lgalabru]

I came up with a solution for this with #240, an example is described here: friedger/clarity-catamaranswaps#2.
Would love to get your thoughts on this approach @MarvinJanssen and @LNow!

[LNow]

@lgalabru this look very nice.
Would it be possible to add "super advanced" version of it, let's say manualSetup in which users would have the possibility to manually control order of deployed contracts and block-height at which they are deployed?

manualSetup: async (chain: Chain, accounts: Map<string, Account>, contracts: Map<string, Contract>) => {
    const deployer = accounts.get("deployer")!;
   chain.mineEmptyBlock(400);

    chain.mineBlock([
      Tx.deployContract("cool-contract", contracts.get("cool-contract")!.code, deployer.address)
    ]);
 
    chain.mineBlock([
      Tx.contractCall("cool-contract", "init", [], deployer.address)
    ]);
    chain.mineEmptyBlock(10);

   chain.mineBlock([
      Tx.deployContract("another-contract", contracts.get("another-contract")!.code, deployer.address)
    ]);
  },

It would help simulate more realistic deployments.

[unclemantis]

Digging it!

…

On Wed, Feb 9, 2022, 10:00 AM LNow ***@***.***> wrote: @lgalabru <https://github.com/lgalabru> this look very nice. Would it be possible to add "super advanced" version of it, let's say manualSetup in which users would have the possibility to manually control order of deployed contracts and block-height at which they are deployed? manualSetup: async (chain: Chain, accounts: Map<string, Account>, contracts: Map<string, Contract>) => { const deployer = accounts.get("deployer")!; chain.mineEmptyBlock(400); chain.mineBlock([ Tx.deployContract("cool-contract", contracts.get("cool-contract")!.code, deployer.address) ]); chain.mineBlock([ Tx.contractCall("cool-contract", "init", [], deployer.address) ]); chain.mineEmptyBlock(10); chain.mineBlock([ Tx.deployContract("another-contract", contracts.get("another-contract")!.code, deployer.address) ]); }, It would help simulate more realistic deployments. — Reply to this email directly, view it on GitHub <#38 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAFPO3LV73PNH5AY64LUZ63U2J6SBANCNFSM46LMGWGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[lgalabru]

Addressed in v0.28.0