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

Sending multiple packets from one tansaction (or msg?) not supported #255

Closed
webmaster128 opened this issue Oct 19, 2022 · 8 comments · Fixed by #258
Closed

Sending multiple packets from one tansaction (or msg?) not supported #255

webmaster128 opened this issue Oct 19, 2022 · 8 comments · Fixed by #258

Comments

@webmaster128
Copy link
Member

I recognized issues when multiple packets are sent from one transaction. Turns out the log string we get from Tendermint is buggy (see tendermint/tendermint#9595). So most likely the code here is correct, but I want to bring this up to help other users trying to debug their issues.

Some ideas to improve the situation:

  • Ensure my bug report in Tendermint is valid and this is not a CosmWasm or wasmd bug
  • Use the events array instead of the raw log to work around it
@webmaster128
Copy link
Member Author

The place where this pops up is:

  private async getPacketsFromTxs({
    minHeight,
    maxHeight,
  }: QueryOpts = {}): Promise<PacketWithMetadata[]> {
    let query = `send_packet.packet_connection='${this.connectionID}'`;
    if (minHeight) {
      query = `${query} AND tx.height>=${minHeight}`;
    }
    if (maxHeight) {
      query = `${query} AND tx.height<=${maxHeight}`;
    }

    const search = await this.client.tm.txSearchAll({ query });
    const resultsNested = search.txs.map(({ hash, height, result }) => {
      const parsedLogs = logs.parseRawLog(result.log);
      // we accept message.sender (cosmos-sdk) and message.signer (x/wasm)
      let sender = '';
      try {
        sender = logs.findAttribute(parsedLogs, 'message', 'sender').value;
      } catch {
        try {
          sender = logs.findAttribute(parsedLogs, 'message', 'signer').value;
        } catch {
          this.client.logger.warn(
            `No message.sender nor message.signer in tx ${toHex(hash)}`
          );
        }
      }
      return parsePacketsFromLogs(parsedLogs).map((packet) => ({
        packet,
        height,
        sender,
      }));
    });
    return ([] as PacketWithMetadata[]).concat(...resultsNested);
  }

@ethanfrey
Copy link
Member

Use the events array instead of the raw log to work around it

This would make it worse most likely, as it would merge the multiple packet events of the same type into one packet. Using raw_log json should keep them as independent events.

Can you attach the JSON out of a raw_log where two ibc packets were emitted in the same tx? It may be a bug in parsing. Having a fixed json to test against would allow us to debug this much quicker

@webmaster128
Copy link
Member Author

This would make it worse most likely, as it would merge the multiple packet events of the same type into one packet. Using raw_log json should keep them as independent events.

In the events they are separate. In the logs they are merged. See tendermint/tendermint#9595. There you also find all the example outputs.

@ethanfrey
Copy link
Member

Good to discuss with tendermint/sdk teams.
Not sure the proper way to get anything out of that data.

NB: what is the events field you refer to? Can you link to eg mintscan to show how this is different than log or raw_log?

@webmaster128
Copy link
Member Author

In https://gist.github.com/webmaster128/5efcb9de025f05a1cd2d55c22e25eb00 you see the full Tendermint RPC response of this transaction: https://testnet.mintscan.io/juno-testnet/txs/502E6F4AEA3FB185DD894D0DC14E013C45E6F52AC00A0B5224F6876A1CA107DB.
There we get .result.tx_result.log which IMO contains buggy events. This is what we use for packets from tx search. Then there is .result.tx_result.events, which contains the events rolled out and complete. Since the events are binary in Tendeermint 0.34, they are base64 encoded here.

The term "raw log" does not exist in Tendermint. It was introduced at a higher level (the old LCD API?) there Tendermint's log was parsed into log and copied into raw_log. We inherited that in CosmJS when propagating Tendermint data to higher levels: rawLog: tx.result.log || "",.

@ethanfrey
Copy link
Member

Yes, we should use (and properly parse) tendermint events. This seems to have improved since 2 years ago, when those were not very reliable.

Are these returned / processed properly in CosmJS? If they are, let's use them. If not, let's add those to next CosmJS release and then add them. I think we only support modern tendermint rpc endpoints now, so this should be a safe change. We support tendermint 0.34 (and soon 0.37?)

@ethanfrey
Copy link
Member

Okay, it seems that BroadcastTxCommitResponse.deliverTx includes [TxData.events] (https://github.com/cosmos/cosmjs/blob/51a7214d0117351529bed7342078be066d489d5e/packages/tendermint-rpc/src/tendermint34/responses.ts#L200)

It would be good to update ts-relayer to use this, if this is now most reliable source of data.

I never really likes raw_log parsing, but that was the best source a while ago

@webmaster128
Copy link
Member Author

I think that can be done. I'll check it out.

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

Successfully merging a pull request may close this issue.

2 participants