Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not having call for transaction method is a mistake. #57

Open
hickscorp opened this issue Jul 20, 2019 · 3 comments
Open

Not having call for transaction method is a mistake. #57

hickscorp opened this issue Jul 20, 2019 · 3 comments

Comments

@hickscorp
Copy link

hickscorp commented Jul 20, 2019

The fact that a contract.methods.aTransactionMethod(args...).send() is available but not a contract.methods.aTransactionMethod(args...).call() is a big problem for us, as it is very common practice to run a call "blank" before performing a real send transaction.

The reason behind this sort of choice is that revert messages cannot be read when they occur on a send - they can only be read when using call using direct bytecode ABI encoding.

An example of what this led us to do:

const REVERT_FUNCTION_ID = "0x08c379a0";

...

  safeSendTx = async (state: State, opts: any) => {
    if (!isInitialized(state)) {
      throw new Error("The provider isn't ready to relay transactions.");
    }
    // First, simulate the transaction for free.
    await this.simulateTxSend(state, opts);
    // If we're here, it's unlikelly that a revert will occur, as the simulation
    // didn't throw.
    const tx = await state.eth.sendTransaction(opts);
    const hash = await tx.getTxHash();
    try {
      return await tx.getReceipt();
    } catch (ex) {
      // Get some info about the failed transaction hash.
      const fullTx = await state.eth.getTransaction(hash);
      // As the transaction failed, we will try and simulate it again in the block
      // it failed to try and retrieve a "revert" message.
      await this.simulateTxSend(opts, fullTx.blockNumber || undefined);
      throw new Error("The transaction failed without a revert message.");
    }
  };

  simulateTxSend = async (state: State, opts: any, block?: BlockType) => {
    if (!isInitialized(state)) {
      throw new Error("The provider isn't ready to relay transactions.");
    }
    const txString = await state.eth.call(opts, block);
    if (!txString.startsWith(REVERT_FUNCTION_ID)) {
      return txString;
    }
    const enc = txString.substring(REVERT_FUNCTION_ID.length);
    const dec = abiCoder.decodeParameter("string", enc);
    throw new Error(dec);
  };

We have to call it like this:

          api
            .safeSendTx(state, {
              ...utils.chainGasOpts,
              data: transact.methods.approve(account, i).encodeABI(),
              from: state.account,
              to: utils.chainManifest.transact,
            })

This is really cumbersome - it would be great to have access to call even on transaction methods exactly like web3 and truffle do - allowing for shooting "blanks".

Please expose the option to call methods of contracts that are transactions.

@xf00f
Copy link
Owner

xf00f commented Jul 20, 2019

I can look into this for next patch release.

In the meantime, the call method should be present on the object, even if not exposed through the TypeScript interface. Underlying it's a Tx object:
https://github.com/xf00f/web3x/blob/master/web3x/src/contract/tx.ts#L68

So with appropriate casting you should be able to use it without tonnes of hackery.

@hickscorp
Copy link
Author

hickscorp commented Jul 21, 2019

@xf00f Thank you for the feedback.

contract.methods.doStuff(...) gives me a TxSend<T>. How would I cast that into a TxCall<T>? The only way I found so far is:

// Inside a generic defining T from the original (method : TxSend<T>):
const callMethod : TxCall<T> = method as any;

Is this what you meant? I'm not sure - as if the underlying implementation changes, this would still compile and fail only at runtime...

@uhhhh2
Copy link

uhhhh2 commented Sep 18, 2019

Would it be a good idea to change the references to TxSend in the web3x-codegen project to either of:

  • Tx<${name}TransactionReceipt> (https://github.com/xf00f/web3x/blob/v4.0.4/web3x/src/contract/tx.ts#L68)
  • A new interface (TxSendAndCall<${name}TransactionReceipt>? TxSendWithLocalCallCheck<${name}TransactionReceipt>? something else?) that inherits from TxSend and TxCall interfaces like the Tx class, but includes some indication of the intended use (such as comments suggesting that developers use the TxCall portions to check if the call would work locally, and then use the TxSend portions to actually call the method).

The references to TxSend:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants