patractlabs / redspot

Redspot is an Substrate pallet-contracts (ink!) development environment. Compile your contracts and run them on a different networks. Redspot's core forks from Hardhat but changed a lot to suit substrate.
https://redspot.patract.io/
Other
67 stars 22 forks source link

[feature request?] redspot-chai - detect Errors returned from blockchain #78

Closed RoyTimes closed 3 years ago

RoyTimes commented 3 years ago

It seems like the only way to catch an error thrown by blockchain is by the fact that the blockchain does not emit a success event.

For the following code: if someFunc is executed successfully, it will emit an event SomeSuccessEvent. If failed, depends on the underlying logics of someFunc, it can throw Error::AccessDenied or Error::NotFound

For instance, when sender1 is not authorized to execute someFunc, the smart contract shall emit Error::AccessDenied, but the only way to capture it seems to be trying to verify that the blockchain does not emit SomeSuccessEvent.

await expect(contract.tx.someFunc(param, { signer: sender1 }))
  .to.not.emit(contract, 'SomeSuccessEvent')
  .withArgs(0, sender1.address)

Let me know if I'm mistaken and how to do it the right way. Otherwise, I would very much to see redspot-chai implement such functionality.

ii-ii-ii commented 3 years ago

Have you tried expect(func).to.throw(error) or expect(func).to.throw()

expect(() => {
    contract.tx.someFunc(param, { signer: sender1 })
}).to.throw()
ii-ii-ii commented 3 years ago
const result = await contract.tx.someFunc(param, { signer: sender1 })
result.error // Is it possible to get the result? Can `Error::AccessDenied ` be determined by `result.error`?
RoyTimes commented 3 years ago

Hi @ii-ii-ii

Thanks for the suggestions. I was trying to figure this out about a month ago, and tried these methods but, unfortunately, they do not work. So I cheated and tried checking .to.not.emit instead. As you can see, when I was submitting a milestone to Web3 Foundation, it seems to be a matter that deserves more investigations.

I tried a bunch of stuff. To set the scene: someFunc changes blockchain state and it returns Result<(), Error>, when an user is not authorized to exec, it returns Error::AccessDenied

// Types Definations .. 
Error: {
  _enum: [ 'AccessDenied', 'OtherError']
},

First Try

// ..... Testing Template Code in smartcontract.test.ts
it('test error catching', async() => {
  const { contract, sender1, sender2 } = await setup();
  await expect(contract.tx.someFunc(...parms, { signer: unauthorizedSigner }))
    .to.not.emit(contract, 'Success')
})

Result Success not emitted. Test passes

Second Try

// ..... Testing Template Code in smartcontract.test.ts
it('test error catching', async() => {
  const { contract, sender1, sender2 } = await setup();
  try {
      const result = await contract.tx.someFunc(...parms, { signer: unauthorizedSigner })
      console.log(result)
    } catch(err) {
      console.error('error', err)
    }
})

Result

{
  from: '5Hj1wzRgfSDs284iuFnUftz1Jd6iG53RknetMd9hbyQyzuPh',
  txHash: '0x77fed7ebf60bda2f60622098c222e2e44e9dfb3979c2f525d695b1a63a8c177d',
  blockHash: '0x78121d25bc943a1be80754f3e5d950aef1c34f486ff1c9464a928569e7ac9fa7',
  result: SubmittableResult {
    dispatchError: undefined,
    dispatchInfo: Type(3) [Map] {
      'weight' => [Type],
      'class' => [Type],
      'paysFee' => [Type],
      registry: TypeRegistry {},
      weight: [Getter],
      class: [Getter],
      paysFee: [Getter]
    },
    events: [ [Type [Map]] ],
    status: Type {
      registry: TypeRegistry {},
      isFuture: [Getter],
      asFuture: [Getter],
      isReady: [Getter],
      asReady: [Getter],
      isBroadcast: [Getter],
      asBroadcast: [Getter],
      isInBlock: [Getter],
      asInBlock: [Getter],
      isRetracted: [Getter],
      asRetracted: [Getter],
      isFinalityTimeout: [Getter],
      asFinalityTimeout: [Getter],
      isFinalized: [Getter],
      asFinalized: [Getter],
      isUsurped: [Getter],
      asUsurped: [Getter],
      isDropped: [Getter],
      asDropped: [Getter],
      isInvalid: [Getter],
      asInvalid: [Getter]
    }
  },
  events: undefined
}

Nothing is thrown. result.status.isInBlock == true result.status.isFinalized == false result.error == undefined
events == undefined

Third Try

// ..... Testing Template Code in smartcontract.test.ts
it('test error catching', async() => {
  const { contract, sender1, sender2 } = await setup();
  try {
      const result = await contract.query.someFunc(...parms, { signer: unauthorizedSigner })
      console.log('unauthorizedSigner', result.output?.toHuman())
    } catch(err) {
      console.error('error', err)
    }

    try {
      const result = await contract.query.someFunc(...parms, { signer: authorizedSigner })
      console.log('authorizedSigner', result.output?.toHuman())
    } catch (err) {
      console.error('error', err)
    }
})

Result

unauthorizedSigner { Err: 'AccessDenied' }
authorizedSigner { Ok: [] }

However, the blockchain state is not mutated. No changes is written to the smart contract storage.

Conclusion?

My guess is that because of the SubmittableResult design by Polkadot.js that requires passing in a callback hook that updates status, somehow the returned value is well populated on tx function, while, query function does. I am not sure what I'm dealing with right now.

So, I guess two possible solutions:

  1. Make it dirty: when redspot is running tx, do a query first to get the returned value and insert into the result ... this is very hacky but I guess will solve this issue.
  2. Try to populate the returned value after isInBlock is set to be true somehow. I guess this is gonna require some digging into the source code of polkadot.js.
atenjin commented 3 years ago

This is related to https://github.com/paritytech/ink/issues/641 It seems that you already know the relationship between "the error of transaction" and "the error of contract logic". Currently chai could only detect the "the error of transaction". And the "the error of contract logic" should be supported by pallet-contracts, just like what I comment in ink#641.

Thus currently chai could only do like solidity testcase. As a common convention, we only print the event of the contract in the branch where the logic is executed correctly. Exceptions generally use assert to interrupt the execution of the contract or return in an error form, but the returned branch does not print the event.

Thus if call current transaction do not print the related event, then it must meet an error. However this way do not know the reason of the error, and not like ethereum EVM, the assert string do not print in Wasm executor.

The final way to resolve this thing is waiting ink fix issue#641, need a protocol for pallet-contracts and ink!&ask! or other contract language framework. But for now, I think the dirty way is your first method, do a mock call to execute the transaction (In fact for 3rd parity, for example a wallet or something else could do this method).

We do not think the mock call should be integrated into chai, the developers should write this logic by themself requirements. And we advice this:

it('test error catching', async() => {
  const { contract, sender1, sender2 } = await setup();
  await expect(contract.tx.someFunc(...parms, { signer: unauthorizedSigner }))
    .to.not.emit(contract, 'TheFuncExpectedContractEvent')
// then if the event do not emit, do query to do the mock call
//  await contract.query.someFunc(...parms, { signer: unauthorizedSigner })
})
RoyTimes commented 3 years ago

Dirty method it is. Closing this issue for now.