Merge branch 'soroban/xcall-multi' of github.com:icon-project/xcall-m…

…ulti into soroban/xcall-multi

contracts/soroban/contracts/xcall/src/dapp.rs

@@ -40,7 +40,6 @@ pub fn try_handle_call_message(
             event::call_executed(&e, req_id, code, String::from_str(&e, "success"));
             code
         }
-        // TODO: convert error type to string
         Err(err) => match err {
             Ok(_error) => {
                 let code = CSResponseType::CSResponseFailure.into();

contracts/soroban/contracts/xcall/src/execute_call.rs

@@ -1,4 +1,4 @@
-use soroban_sdk::{vec, Address, Bytes, Env};
+use soroban_sdk::{vec, Address, Bytes, Env, String};
 use soroban_xcall_lib::messages::msg_type::MessageType;
 
 use crate::{
@@ -46,9 +46,11 @@ pub fn execute_message(
                 &data,
                 req.protocols().clone(),
             );
+
+            let code: u8 = CSResponseType::CSResponseSuccess.into();
+            event::call_executed(&env, req_id, code, String::from_str(&env, "success"));
         }
         MessageType::CallMessageWithRollback => {
-            storage::store_reply_state(&env, &req);
             let code = dapp::try_handle_call_message(
                 &env,
                 req_id,
@@ -57,16 +59,9 @@ pub fn execute_message(
                 &data,
                 req.protocols().clone(),
             );
-            storage::remove_reply_state(&env);
 
             let response_code = code.into();
-            let mut message = Bytes::new(&env);
-            let call_reply = storage::remove_call_reply(&env);
-            if call_reply.is_some() && response_code == CSResponseType::CSResponseSuccess {
-                message = call_reply.unwrap().encode(&env);
-            }
-
-            let result = CSMessageResult::new(req.sequence_no(), response_code, message);
+            let result = CSMessageResult::new(req.sequence_no(), response_code, Bytes::new(&env));
             let cs_message = CSMessage::from_result(&env, &result).encode(&env);
 
             let nid = req.from().nid(&env);

contracts/soroban/contracts/xcall/src/send_message.rs

@@ -46,20 +46,17 @@ pub fn send_call(
     let encode_msg = cs_message.encode(&env);
     helpers::ensure_data_size(encode_msg.len() as usize)?;
 
-    if is_reply(&env, &nid_to, &envelope.sources) && !need_response {
-        storage::store_call_reply(&env, &request);
-    } else {
-        call_connection(
-            &env,
-            &tx_origin,
-            &nid_to,
-            sequence_no,
-            envelope.sources,
-            need_response,
-            encode_msg.clone(),
-        )?;
-        claim_protocol_fee(&env, &tx_origin)?;
-    }
+    call_connection(
+        &env,
+        &tx_origin,
+        &nid_to,
+        sequence_no,
+        envelope.sources,
+        need_response,
+        encode_msg.clone(),
+    )?;
+    claim_protocol_fee(&env, &tx_origin)?;
+
     event::message_sent(&env, sender, to.to_string(), sequence_no);
 
     Ok(sequence_no)
@@ -134,10 +131,6 @@ pub fn get_total_fee(
     sources: Vec<String>,
     rollback: bool,
 ) -> Result<u128, ContractError> {
-    if !rollback && is_reply(&env, &nid, &sources) {
-        return Ok(0_u128);
-    }
-
     let mut sources = sources;
     if sources.is_empty() {
         let default_conn = storage::default_connection(&env, nid.clone())?;
@@ -155,25 +148,3 @@ pub fn get_total_fee(
 
     Ok(connections_fee + protocol_fee)
 }
-
-pub fn is_reply(e: &Env, nid: &String, sources: &Vec<String>) -> bool {
-    if let Some(req) = storage::get_reply_state(&e) {
-        if req.from().nid(&e) != *nid {
-            return false;
-        }
-        return are_array_equal(req.protocols(), &sources);
-    }
-    false
-}
-
-pub fn are_array_equal(protocols: &Vec<String>, sources: &Vec<String>) -> bool {
-    if protocols.len() != sources.len() {
-        return false;
-    }
-    for protocol in protocols.iter() {
-        if !sources.contains(protocol) {
-            return false;
-        }
-    }
-    return true;
-}

contracts/soroban/contracts/xcall/src/storage.rs

@@ -115,10 +115,6 @@ pub fn get_proxy_request(e: &Env, req_id: u128) -> Result<CSMessageRequest, Cont
     request
 }
 
-pub fn get_reply_state(e: &Env) -> Option<CSMessageRequest> {
-    e.storage().temporary().get(&StorageKey::ReplyState)
-}
-
 pub fn get_pending_request(e: &Env, hash: BytesN<32>) -> Vec<String> {
     let key = StorageKey::PendingRequests(hash);
     let pending_request = e.storage().persistent().get(&key).unwrap_or(Vec::new(&e));
@@ -139,13 +135,6 @@ pub fn get_pending_response(e: &Env, hash: BytesN<32>) -> Vec<String> {
     pending_response
 }
 
-pub fn get_call_reply(e: &Env) -> Option<CSMessageRequest> {
-    e.storage()
-        .temporary()
-        .get(&StorageKey::CallReply)
-        .unwrap_or(None)
-}
-
 pub fn get_own_network_address(e: &Env) -> Result<NetworkAddress, ContractError> {
     let config = get_config(&e)?;
     let from = NetworkAddress::new(
@@ -206,27 +195,6 @@ pub fn remove_proxy_request(e: &Env, req_id: u128) {
         .remove(&StorageKey::ProxyRequest(req_id))
 }
 
-pub fn store_call_reply(e: &Env, reply: &CSMessageRequest) {
-    let key = StorageKey::CallReply;
-    e.storage().temporary().set(&key, reply);
-}
-
-pub fn remove_call_reply(e: &Env) -> Option<CSMessageRequest> {
-    let call_reply = get_call_reply(&e);
-    e.storage().temporary().remove(&StorageKey::CallReply);
-
-    call_reply
-}
-
-pub fn store_reply_state(e: &Env, req: &CSMessageRequest) {
-    let key = StorageKey::ReplyState;
-    e.storage().temporary().set(&key, req);
-}
-
-pub fn remove_reply_state(e: &Env) {
-    e.storage().temporary().remove(&StorageKey::ReplyState)
-}
-
 pub fn store_pending_request(e: &Env, hash: BytesN<32>, sources: &Vec<String>) {
     let key = StorageKey::PendingRequests(hash.clone());
     e.storage().persistent().set(&key, sources);

contracts/soroban/contracts/xcall/src/test/contract.rs

@@ -8,7 +8,7 @@ use soroban_sdk::{
 extern crate std;
 
 use super::setup::*;
-use crate::{contract::XcallClient, storage};
+use crate::contract::XcallClient;
 
 #[test]
 fn test_initialize() {
@@ -155,20 +155,3 @@ fn test_get_fee() {
     let fee = client.get_fee(&ctx.nid, &need_response, &Some(sources));
     assert_eq!(fee, protocol_fee + centralized_conn_fee)
 }
-
-#[test]
-fn test_get_fee_should_returns_zero_in_reply_state() {
-    let ctx = TestContext::default();
-    let client = XcallClient::new(&ctx.env, &ctx.contract);
-    ctx.init_context(&client);
-
-    let need_response = false;
-    let req = get_dummy_message_request(&ctx.env);
-    let sources = req.protocols().clone();
-
-    ctx.env
-        .as_contract(&ctx.contract, || storage::store_reply_state(&ctx.env, &req));
-
-    let fee = client.get_fee(&ctx.nid, &need_response, &Some(sources));
-    assert_eq!(fee, 0_u128);
-}

contracts/soroban/contracts/xcall/src/test/send_message.rs

@@ -5,7 +5,7 @@ extern crate std;
 use soroban_sdk::{
     bytes, symbol_short,
     testutils::{Address as _, AuthorizedFunction, AuthorizedInvocation},
-    vec, Address, Bytes, IntoVal, String, Vec,
+    vec, Address, Bytes, IntoVal, String,
 };
 use soroban_xcall_lib::messages::{
     call_message::CallMessage, call_message_rollback::CallMessageWithRollback, envelope::Envelope,
@@ -343,73 +343,3 @@ fn test_claim_protocol_fail_for_insufficient_amount_sent() {
         send_message::claim_protocol_fee(&ctx.env, &sender).unwrap();
     });
 }
-
-#[test]
-fn test_array_equal_for_mismatch_length() {
-    let ctx = TestContext::default();
-
-    let protocols = get_dummy_sources(&ctx.env);
-    let sources: Vec<String> = vec![&ctx.env];
-
-    ctx.env.as_contract(&ctx.contract, || {
-        let res = send_message::are_array_equal(&protocols, &sources);
-        assert_eq!(res, false)
-    });
-}
-
-#[test]
-fn test_array_equal_returns_false_for_unknown_protocol() {
-    let ctx = TestContext::default();
-
-    let protocols = get_dummy_sources(&ctx.env);
-    let sources: Vec<String> = vec![
-        &ctx.env,
-        String::from_str(&ctx.env, "layerzero"),
-        String::from_str(&ctx.env, "wormhole"),
-    ];
-
-    ctx.env.as_contract(&ctx.contract, || {
-        let res = send_message::are_array_equal(&protocols, &sources);
-        assert_eq!(res, false)
-    });
-}
-
-#[test]
-fn test_array_equal() {
-    let ctx = TestContext::default();
-
-    let protocols = get_dummy_sources(&ctx.env);
-    let sources = get_dummy_destinations(&ctx.env);
-
-    ctx.env.as_contract(&ctx.contract, || {
-        let res = send_message::are_array_equal(&protocols, &sources);
-        assert_eq!(res, true)
-    });
-}
-
-#[test]
-fn test_is_reply_for_mismatch_network() {
-    let ctx = TestContext::default();
-
-    ctx.env.as_contract(&ctx.contract, || {
-        let req = get_dummy_message_request(&ctx.env);
-        storage::store_reply_state(&ctx.env, &req);
-
-        let nid = String::from_str(&ctx.env, "icon");
-        let res = send_message::is_reply(&ctx.env, &nid, req.protocols());
-        assert_eq!(res, false)
-    });
-}
-
-#[test]
-fn test_is_reply_returns_false_when_missing_reply_state() {
-    let ctx = TestContext::default();
-
-    let sources = get_dummy_sources(&ctx.env);
-    let nid = String::from_str(&ctx.env, "icon");
-
-    ctx.env.as_contract(&ctx.contract, || {
-        let res = send_message::is_reply(&ctx.env, &nid, &sources);
-        assert_eq!(res, false)
-    });
-}

contracts/soroban/contracts/xcall/src/types/storage_types.rs

@@ -10,9 +10,7 @@ pub enum StorageKey {
     SuccessfulResponses(u128),
     Sn,
     Rollback(u128),
-    CallReply,
     ProxyRequest(u128),
-    ReplyState,
     PendingRequests(BytesN<32>),
     PendingResponses(BytesN<32>),
     LastReqId,

docs/stellar-xcall.md

@@ -2,7 +2,7 @@
 
 ## Overview
 
-This document outlines the major changes in the xCall implementation on the Stellar blockchain and the reasoning behind these updates. Key modifications include a new fee deduction mechanism where xCall and each connection directly deduct fees from the user’s account, and an updated storage management approach utilizing instance and persistent storage while extending rent periodically to comply with Stellar's requirements.
+This document outlines the key updates made to the xCall implementation on the Stellar blockchain. These changes were introduced to improve how the protocol handles fees, manage storage more efficiently, and adapt to Stellar’s security model. The goal is to ensure smooth functionality while aligning with Stellar’s unique constraints and requirements.
 
 ## Major Changes
 
@@ -30,6 +30,17 @@ This document outlines the major changes in the xCall implementation on the Stel
 
 - Stellar’s storage model has distinct types such as temporary, persistent, and instance storage, each with different cost and management implications. We use these types based on their suitability and extend the rent periodically as needed to ensure compliance with Stellar's requirements and maintain data accessibility.
 
+**3. Remove Reply State**
+
+**Change**
+
+- The send_call and execute_call methods have been simplified by removing the reply logic
+- When xCall invokes a dApp's method, the dApp is no longer able invoke any further methods back to xCall within the same transaction
+
+**Rationale**
+
+- Stellar's security model restricts cross-contract reentrancy, preventing dApps from re-invoking xCall during the same execution flow. This ensures a more secure and predictable execution environment.
+
 ## Conclusion
 
 The updated implementation of xCall on Stellar deviates from the original architecture but maintains functional equivalence, ensuring that the protocol remains robust and efficient within Stellar's unique constraints.