Make client code call into the new ABCI queries tx endpoints #619
Conversation
sug0
force-pushed
the
tiago/ethbridge/impl-events-endpoint-client-tx
branch
from
1e50da7
to
7d42d0f
Compare
…dge/impl-events-endpoint-add-endpoint
sug0
force-pushed
the
tiago/ethbridge/impl-events-endpoint-client-tx
branch
from
7d42d0f
to
d703fa1
Compare
| impl TxResponse { | ||
| /// Convert an [`Event`] to a [`TxResponse`], or error out. | ||
| pub fn from_event(event: Event) -> Self { | ||
| event.try_into().unwrap_or_else(|err| { | ||
| eprintln!("Error fetching TxResponse: {err}"); | ||
| safe_exit(1); | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
We shouldn't need this, we can just use .try_from() and handle/log the error as appropriate. Having these safe_exits is especially pernicious as it breaks the test runner, if this code ever ends up being used in a Rust test (this is the root cause of #104 )
There was a problem hiding this comment.
To do later - anoma/anoma#1049 - these safe_exits need to be removed
There was a problem hiding this comment.
This sounds like an issue with the test runner. It is supposed to watch the exit status and return an error if it gets a non-zero value.
There was a problem hiding this comment.
It would be good if the test runner could catch that, but it's still bad that we have these functions maybe taking a decision to terminate the process
| eprintln!("Transaction status query deadline of {deadline:?} exceeded"); | ||
| }) | ||
| .and_then(|result| result) | ||
| .unwrap_or_else(|_| cli::safe_exit(1)) |
There was a problem hiding this comment.
Return Err here instead of exiting? The caller could then log the error and decide to exit or pass on the responsibility. Or at least a None or false or something which indicated we didn't succeed.
| let initialized_accounts = event | ||
| .get("initialized_accounts") | ||
| .map(String::as_str) | ||
| // TODO: fix finalize block, to return initialized accounts, | ||
| // even when we reject a tx? | ||
| .or(Some("[]")) | ||
| // NOTE: at this point we only have `Some(vec)`, not `None` | ||
| .ok_or_else(|| unreachable!()) | ||
| .and_then(|initialized_accounts| { | ||
| serde_json::from_str(initialized_accounts) | ||
| .map_err(|err| format!("JSON decode error: {err}")) | ||
| })?; |
There was a problem hiding this comment.
What do we expect event.get("initialized_accounts") to look like now? It could be useful to factor out and have a test specifically for this parsing workaround we're doing. Is this an issue with Tendermint?
There was a problem hiding this comment.
no it's an issue with our finalize block implementation. at first I thought it was tendermint's fault, but it's namada code's. when we reject a tx, initialized_accounts is not set in the Event attributes, so we need to do this crap, which is a bit unfortunate.
There was a problem hiding this comment.
Ah right, I made an issue - #643 , we can eventually fix it and simplify this
sug0
force-pushed
the
tiago/ethbridge/impl-events-endpoint-client-tx
branch
from
dab19ad
to
e0c93da
Compare
sug0
force-pushed
the
tiago/ethbridge/impl-events-endpoint-client-tx
branch
from
b2aece9
to
92dec57
Compare
|
@james-chf left the |
Closes #636 Co-authored-by: satan <jacob.turner870@gmail.com>
sug0
force-pushed
the
tiago/ethbridge/impl-events-endpoint-client-tx
branch
from
1d9d328
to
d923df6
Compare
- Add atoms for accounts, balances, chain config, fiat rates, fiat currency setting, and temporary redux store atom - Fetch balances, accounts etc. for jotai as well as redux - Use jotai instead of redux for displaying data in DerivedAccounts - Remove redux coins slice - Calculate total balance in AccountOverview instead of DerivedAccounts - Display "-" instead of "0" when balance for an account is loading - Refactor DerivedAccounts with smaller FiatBalanceDisplay component - Only fetch fiat rates when DerivedAccounts is first mounted and remove apparently broken timestamp code
Based on #610
Implements the fourth task of issue #563