ecadlabs / taquito

A library for building dApps on the Tezos Blockchain - JavaScript / TypeScript
https://taquito.io
Apache License 2.0
298 stars 116 forks source link

operationResults doesn't truly reflect chain execution result #2602

Open hui-an-yang opened 1 year ago

hui-an-yang commented 1 year ago

Description

Our current design of operationResults is using the preapplyOperations response while it works for many cases but when it comes to 2 contract calls in the same block the first one modify the storage and the second contract call will hit failWith in contract due to the first contract call change our second operationResults still reflect it's preapplyResponse which the operation succeed.

Steps To Reproduce

run('https://ghostnet.ecadinfra.com/')

async function run(url: string) {
  try {
    const Tz1 = new TezosToolkit(url)
    Tz1.setSignerProvider(new InMemorySigner('secret_key_1'))
    const Tz2 = new TezosToolkit(url)
    Tz2.setSignerProvider(new InMemorySigner('secret_key_2'))

    const contract1 = await Tz1.contract.at('KT1LU9Nd7f1z8jcSyUWtDkGgdYSnLMTzwhY9') // taco shop contract
    const contract2 = await Tz2.contract.at('KT1LU9Nd7f1z8jcSyUWtDkGgdYSnLMTzwhY9') // taco shop contract

    let storage: BigNumber = await contract1.storage()

    if (storage.toNumber() < 10) {
      let op0 = await contract1.methods.increment(10).send();
      await op0.confirmation()
      console.log(op0.status)
    }

    let op1 = await contract1.methods.decrement(10).send();
    let op2 = await contract2.methods.decrement(8).send();

    op2.confirmation()
    op1.confirmation()

    console.log('op1', JSON.stringify(op1.operationResults))
    console.log('op2', JSON.stringify(op2.operationResults))

} catch (e) {
    console.log(JSON.stringify(e))
  }
}

Expected behavior op2 operationResults should throw an operationError that it hit failwith in contract because of the storage doesn't have enough to be deducted.

image

Screenshots Our current design made op2 operationResults shows the contract call succeed which isn't correct on chain and indexer image

Additional context

We might want to reference what octez-client is doing after injecting for example. Here's a link for reference

hui-an-yang commented 7 months ago

Tried changing operationResults to get operationObject from confirmation() in Operation parent class, it added complexity to unit test of each operation classes. The operation class is not pure and we'll need to mock rxjs observable results