[r2r]Tx wait for confirmation error return fix #1443
Conversation
borngraced
changed the title
mm2src/coins/utxo/rpc_clients.rs
Outdated
| @@ -173,6 +173,12 @@ impl UtxoRpcClientEnum { | |||
| } | |||
| }, | |||
| Err(e) => { | |||
| if matches!(&e.get_inner(), UtxoRpcError::Transport(_)) | |||
| && e.to_string().contains("No information available about transaction") | |||
There was a problem hiding this comment.
It's not a good practice to check for a certain string in the error returned since this string can be changed in blockchain code and/or can be different for different coins, for instance this message can be returned from the bitcoin daemon https://github.com/bitcoin/bitcoin/blob/c5ba1d92b633e703c25121c0d73dc0e04baae01b/src/rpc/rawtransaction.cpp#L260
It's better to check for error codes instead. Fortunately, we already have such check since this is the error code for a missing transaction https://github.com/KomodoPlatform/atomicDEX-API/blob/317510d9a85ba6ca3aa71831d0494ca52444c3d9/mm2src/coins/utxo/rpc_clients.rs#L70
and there is a function that returns a transaction if it's on-chain or None otherwise https://github.com/KomodoPlatform/atomicDEX-API/blob/317510d9a85ba6ca3aa71831d0494ca52444c3d9/mm2src/coins/utxo/rpc_clients.rs#L355
I think you will need to implement another function similar to the above one for verbose transactions and use it instead of get_verbose_transaction.
Also to check for this error code for Transport error, I think you will need to refactor JsonRpcErrorType :: Transport https://github.com/KomodoPlatform/atomicDEX-API/blob/317510d9a85ba6ca3aa71831d0494ca52444c3d9/mm2src/common/jsonrpc_client.rs#L245
And this part of transport fn
https://github.com/KomodoPlatform/atomicDEX-API/blob/317510d9a85ba6ca3aa71831d0494ca52444c3d9/mm2src/coins/utxo/rpc_clients.rs#L670-L679
There was a problem hiding this comment.
Do I really need to do this? Since I can just create a helper function to check for this instead of adding new trait method?
I think you will need to implement another function similar to the above one for verbose transactions and use it instead of get_verbose_transaction.
There was a problem hiding this comment.
I refactored the error type for Transport and added an helper function to check for the error code instead
borngraced
changed the title
borngraced
changed the title
|
@shamardy it's ready for another round of review |
* refactor p2p message processing flow Signed-off-by: ozkanonur <work@onurozkan.dev> * drop `message` mutability in `process_p2p_message` Signed-off-by: ozkanonur <work@onurozkan.dev> * refactor `fn tx_enum_from_bytes` error type Signed-off-by: Onur Özkan <work@onurozkan.dev> * check topics in `lp_topic_list_validation` Signed-off-by: Onur Özkan <work@onurozkan.dev> * optimize Signed-off-by: Onur Özkan <work@onurozkan.dev> * check if there is any byte left in tx deserialization Signed-off-by: Onur Özkan <work@onurozkan.dev> * add backwards comparison in `fn deserialization` Signed-off-by: Onur Özkan <work@onurozkan.dev> * rollback 78c6810 Signed-off-by: Onur Özkan <work@onurozkan.dev> * set JEMALLOC_SYS_WITH_MALLOC_CONF on build pipelines Signed-off-by: ozkanonur <work@onurozkan.dev> * Update README.md * add `_` to unused variables Signed-off-by: ozkanonur <work@onurozkan.dev> * refactor `lp_topic_list_validation` as `topic_prefix_and_coin_validation` Signed-off-by: ozkanonur <work@onurozkan.dev> * fix reviews Signed-off-by: Onur Özkan <work@onurozkan.dev> * update `tx_enum_from_bytes` and write unit test Signed-off-by: ozkanonur <work@onurozkan.dev> * update `tx_enum_from_bytes` in tendermint Signed-off-by: ozkanonur <work@onurozkan.dev> * update error log of `process_p2p_message` Signed-off-by: Onur Özkan <work@onurozkan.dev> * create `TxMarshalingErr` Signed-off-by: ozkanonur <work@onurozkan.dev> * fix `test_tx_enum_from_bytes` Signed-off-by: ozkanonur <work@onurozkan.dev> * fix test compilation Signed-off-by: ozkanonur <work@onurozkan.dev> Signed-off-by: ozkanonur <work@onurozkan.dev> Signed-off-by: Onur Özkan <work@onurozkan.dev>
…to tx_wait_for_confirmation_fix
I can confirm that
wait_for_confirmationis used correctly in other protocolsfixes #1442