oasisprotocol / explorer

Official explorer for the Oasis Network.
https://explorer.oasis.io
Apache License 2.0
8 stars 8 forks source link

[Bug]: Status of failed consensus deposits is not communicated #528

Open kostko opened 1 year ago

kostko commented 1 year ago

Is there an existing issue for this?

SUMMARY

When a consensus deposit fails, the explorer does not indicate that.

STEPS TO REPRODUCE

  1. Create a deposit transaction in the runtime, e.g. using the CLI, such that the deposit will in fact fail (e.g. because the user does not have enough allowance or balance in the consensus layer). For example I used oasis account deposit 10000.
  2. The CLI will report that the deposit transaction has succeeded, but the deposit has in fact failed (this is only known in the next round):
    Broadcasting transaction...
    Transaction included in block successfully.
    Round:            550814
    Transaction hash: c72d51780c9db7614fb4419d4c82a711ac0c3a430f2737e1f38961d7d019229a
    Execution successful.
    Waiting for deposit result...
    Error: deposit failed with error code 5 from module staking
  3. Looking at the block explorer for this transaction it just shows success as status. While this is technically correct when showing the status of the transaction itself, the deposit itself failed and this is not indicated anywhere on the explorer.

EXPECTED BEHAVIOR

Some indication that the deposit itself failed even though the transaction succeeded.

donouwens commented 7 months ago

After talking with @lukaw3d we thought we could agree on updating the iconography by adding (and switching) the status icons.

So we’d have the success state (we’d invert the colours for the icons), and we’d add the partial success pill (which uses the current success colour scheme). Each would have its dedicated hover. @lukaw3d, during our chat you had a better text for the partial success hover, but I can’t recall it.

Screenshot 2024-02-21 at 18 46 25
lukaw3d commented 7 months ago

@donouwens with current API we can not tell if "but the deposit failed" (an event in the next block). If we could tell then I would argue for red color schema.

more accurate message example: image

kostko commented 7 months ago

I think a "question mark" would be more appropriate in this case. So here the first step will almost always succeed, so as the message (which should be revamped) says the second step's result is actually the important one. Once nexus is able to determine the outcome, we should show this as failed (e.g. red/yellow/orange) or show everything together and say that the operation failed or similar.

donouwens commented 6 months ago

Thank you for the feedback @kostko and @lukaw3d

It makes total sense what you are saying. My only concern is that we’d be reusing the question (if in grey) which would lead to us using the same component to communicate two different messages. The user would not be able to see a distinction between them until they trigger the hover. Therefore, I’d like to propose using an orange (warning-like) question mark in this case.

For the hover, we could perhaps say: “The first step of the deposit was successful, but the second step is unknown.”

Any thoughts?

Screenshot 2024-02-23 at 19 20 49
kostko commented 6 months ago

I think this looks ok. For the message it would be great if we could say (for deposits): "Deposit has started, final result will be known in the next block." or similar. Then instead of "Partial success" it could also say "Started" or similar.