feat(ca_orchestration): implementing chain agnostic orchestration check endpoint #795
Conversation
geekbrother
force-pushed
the
feat/ca_orchestration_check
branch
from
e2071a1 to
6af3a0c
Compare
geekbrother
changed the title
geekbrother
changed the title
src/handlers/chain_agnostic/check.rs
Outdated
| ) | ||
| .await?; | ||
| let transfer_value_stripped = request_payload.transaction.value.trim_start_matches("0x"); | ||
| let transfer_value = U256::from_dec_str(transfer_value_stripped) |
There was a problem hiding this comment.
Should this be from_hex_str not decimal? Didn't look at docs though
src/handlers/chain_agnostic/check.rs
Outdated
| } | ||
|
|
||
| // Check the ERC20 tokens balance for each of supported assets | ||
| for (asset, chains) in BRIDGING_AVAILABLE_ASSETS.entries() { |
There was a problem hiding this comment.
This is a LOT of looping. This will take a long time as well as exhaust a lot of API calls.
There was a problem hiding this comment.
+1 on this. Now we have 3 chains and 1 asset for the demo purposes, however this is very much going to increase in the future in both ways
There was a problem hiding this comment.
We can use our balance API (which is Zerion API actually) to get the available balance without JSON-RPC calls for each contract, but the issue is that the address balance in this case is not the latest (Zerion cache is about 2 minutes) and we are using the JSON-RPC call as well to fetch the latest balance for example for swaps. Not sure how this will impact the UX.
There was a problem hiding this comment.
@geekbrother Balance call is extremely expensive. We should look into minimizing the amount of those.
| ) -> Result<Erc20FunctionType, CryptoUitlsError> { | ||
| // Get the 4 bytes function selector | ||
| let selector: [u8; 4] = function_data[0..4].try_into().map_err(|_| { | ||
| CryptoUitlsError::Erc20DecodeError("Function data is less then 4 bytes.".into()) |
There was a problem hiding this comment.
Should this error be caught by the handler function? If they aren't using ERC-20 transfer should it still require multi chain bridging?
There was a problem hiding this comment.
That's another case: the function selector in the ERC20 contract is 4 bytes, so if the function data is less, that means that's not the ERC20 contract. The function type check is in the handler here and we are returning false for the bridging if that's not a transfer. I don't think we should check the balance if that's not a transfer cc @lukaisailovic
The web example also have this check btw.
There was a problem hiding this comment.
@geekbrother The web code works only for ERC20 transfers and nothing else. Its meant to be used as a showcase
geekbrother
force-pushed
the
feat/ca_orchestration_check
branch
from
6af3a0c to
8e548be
Compare
geekbrother
force-pushed
the
feat/ca_orchestration_check
branch
from
8e548be to
68a1395
Compare
geekbrother
force-pushed
the
feat/ca_orchestration_check
branch
from
68a1395 to
a8568ac
Compare
Description
This PR implements the chain agnostic bridging check endpoint according to the research draft doc.
The endpoint processing the transaction object and making the following checks to return is the bridging required for the transaction:
falsefor the bridging.databy decoding it to the ERC20 contract ABI and check is the function call istransfer. If it's not, returnfalsefor the bridging.truefor the bridging.falseif no bridging assets was found.What's not included:
How Has This Been Tested?
Due Diligence