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.

Sign up for GitHub

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

Jump to bottom

Standardize and improve UX of CCTX error message #3194

Open
lumtis opened this issue Nov 21, 2024 · 1 comment · May be fixed by #3326
Open

Standardize and improve UX of CCTX error message #3194

lumtis opened this issue Nov 21, 2024 · 1 comment · May be fixed by #3326
Assignees
@kingpinXD
Labels
m:crosschain
Milestone
Solana WithdrawAn...

Comments

@lumtis
Copy link
Member

lumtis commented Nov 21, 2024

Describe the Issue

The status message of CCTX was previously split in status_message to describe status and error_message to describe cctx processing error if any.

However, a UX issues still remain:

  • status_message generally doesn't give much valuable information
  • error_message is generall too verbose, not organized enough and contains both error from the smart contract call and revert if there is one.

Example with CCTX 0x61d9517371fa51f3dedc09148b0ab73b24273ef5411e892ad0094438a728ebba on testnet:

    error_message: 'deposit error: contract call failed: method ''depositAndCall'',
      contract ''0xEdf1c3275d13489aCdC6cD6eD246E72458B8795B'', args: [{[116 98 49
      113 97 106 120 113 100 97 118 103 55 113 101 48 115 54 99 48 117 113 53 120
      106 119 109 114 99 110 109 102 108 122 113 107 118 54 119 114 120 106] 0x0000000000000000000000000000000000000000
      18332} 0x65a45c57636f9BcCeD4fe193A602008578BcA90b 63200 0xE869b85987D86d8fBb8913f97A705B4741edE86E
      [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 64 0 0 0 0 0 0 0 0 0 0 0 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 1 0 31 8 114 173 89 103 28 32 216 199 223
      23 218 144 178 97 227 198 29 206 71 31 177 222 58 199 57 100 26 67 128 175 0
      0 0 0 0 0 0 0 0 0 0 0 218 36 85 178 188 153 144 104 42 30 240 191 138 190 201
      21 130 93 66 147 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 54 53 201 173
      197 222 160 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0
      0 128 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 65 46 162
      174 30 134 233 247 163 75 144 111 131 184 223 203 158 35 65 101 101 0 0 145
      201 152 4 164 227 47 62 58 31 93 187 156 163 80 241 90 43 228 232 154 218 235
      128 16 101 154 240 57 245 109 126 166 4 110 64 228 217 198 230 231 192 28 0
      0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]]: execution reverted:
      ret 0x2cdcef78000000000000000000000000e869b85987d86d8fbb8913f97a705b4741ede86e000000000000000000000000da2455b2bc9990682a1ef0bf8abec915825d42930000000000000000000000000000000000000000000000000000000000001b59:
      evm transaction execution failed, processing error: outTxGasFee(127200) more
      than available gas for tx (63200) | Identifiers : tb1qajxqdavg7qe0s6c0uq5xjwmrcnmflzqkv6wrxj-18332-18332-0
      : not enough gas'
    status_message: 'Status changed from PendingOutbound to Aborted: revert failed'

Solution consider:

  • Keep status_message very high level, example "aborted: revert outboundfailed". Can give useful information for abort: cross-chain transaction aborted, asset refunded on ZetaChain on address 0xaaa

  • Split error_message into error_message and revert_error_message

  • Make the error_message less verbose, arg info is not necessary

    • Example: cross-chain call reverted: reason: 0xaaa
    • For known reason the message could be decoded
      • cross-chain call reverted: non-existing target contract
      • cross-chain call reverted: deposit paused
      • cross-chain call reverted: 1 million gas limit reached

  • Error for the revert can be make more explicit, currently we use error message in the protocol that might be unclear to user like outTxGasFee(127200) more than available gas for tx (63200)

    • Example: revert transaction failed: insufficient for revert, needed: 127200, available: 63200
    • revert transaction failed: onRevert call failed

Example, how it can look like for cctx above: (need to investigate why reason is not shown for cctx above)

    error_message: cross-chain call reverted: reason: 0xaaa
    revert_error_message: revert transaction failed: insufficient for revert, needed: 127200, available: 63200
    status_message: cross-chain transaction aborted: revert failed, asset refunded on ZetaChain on address 0xaaa
This was referenced Dec 3, 2024
Open
Merged
@kingpinXD kingpinXD self-assigned this Dec 20, 2024
@kingpinXD
Copy link
Contributor

kingpinXD commented Jan 3, 2025 •
edited
Loading

Currently the error message is generated here like this ,

		errMes := fmt.Sprintf(
			"contract call failed: method '%s', contract '%s', args: %v",
			method,
			contract.Hex(),
			args,
		)

We can write methods to parse this message and break it down into , method , contract args etc

However, I think it would be better to define an Error type for the function CallEVM , something like

type CallEvmError struct {
	Error       string
	Contract    string
	Method      string
	Args        []interface{}
	RevertError string
	Reason      interface{}
}

This would mean the signature of CallEVM becomes

func (k Keeper) CallEVM(
	ctx sdk.Context,
	abi abi.ABI,
	from, contract common.Address,
	value, gasLimit *big.Int,
	commit bool,
	noEthereumTxEvent bool,
	method string,
	args ...interface{},
) (*evmtypes.MsgEthereumTxResponse, CallEvmError)

Alternatively, if the fields like args are not needed, we can remove them from the message altogether.

@kingpinXD kingpinXD linked a pull request Jan 13, 2025 that will close this issue
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kingpinXD kingpinXD

Labels
m:crosschain
Projects
None yet
Milestone
Solana WithdrawAndCall Release (plann...
2 participants
@lumtis @kingpinXD