From 266ad0767e0aef13c7df6d897d8283426661147b Mon Sep 17 00:00:00 2001 From: Jaechang Namgoong Date: Fri, 23 Jun 2023 14:14:38 +0900 Subject: [PATCH 1/2] Redesign xCall executeCall processing to reduce gas costs (#18) Add `_data` field to `CallMessage` eventlog to be reused later by the external caller. Also `executeCall` should be called with `_data`, then xCall compares it with the saved hash value to validate its integrity. --- .../icon/btp/xcall/sample/XCallProxy.java | 4 ++-- .../foundation/icon/btp/xcall/CallService.java | 3 ++- .../icon/btp/xcall/CallServiceEvent.java | 3 ++- .../icon/btp/xcall/CallServiceImpl.java | 18 +++++++++++++----- .../icon/btp/xcall/CallServiceImplTest.java | 14 +++++++++++--- 5 files changed, 30 insertions(+), 12 deletions(-) diff --git a/dapp-sample/src/main/java/foundation/icon/btp/xcall/sample/XCallProxy.java b/dapp-sample/src/main/java/foundation/icon/btp/xcall/sample/XCallProxy.java index 4e4c0a77..ea3d8cee 100644 --- a/dapp-sample/src/main/java/foundation/icon/btp/xcall/sample/XCallProxy.java +++ b/dapp-sample/src/main/java/foundation/icon/btp/xcall/sample/XCallProxy.java @@ -52,7 +52,7 @@ public void executeRollback(BigInteger _sn) { } @Override - public void executeCall(BigInteger _reqId) { - Context.call(this.address, "executeCall", _reqId); + public void executeCall(BigInteger _reqId, byte[] _data) { + Context.call(this.address, "executeCall", _reqId, _data); } } diff --git a/xcall/src/main/java/foundation/icon/btp/xcall/CallService.java b/xcall/src/main/java/foundation/icon/btp/xcall/CallService.java index 669ee229..42447398 100644 --- a/xcall/src/main/java/foundation/icon/btp/xcall/CallService.java +++ b/xcall/src/main/java/foundation/icon/btp/xcall/CallService.java @@ -54,7 +54,8 @@ public interface CallService { * Executes the requested call message. * * @param _reqId The request id + * @param _data The calldata */ @External - void executeCall(BigInteger _reqId); + void executeCall(BigInteger _reqId, byte[] _data); } diff --git a/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceEvent.java b/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceEvent.java index c7e84eb2..56b94193 100644 --- a/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceEvent.java +++ b/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceEvent.java @@ -74,9 +74,10 @@ public interface CallServiceEvent { * @param _to A string representation of the callee address * @param _sn The serial number of the request from the source * @param _reqId The request id of the destination chain + * @param _data The calldata */ @EventLog(indexed=3) - void CallMessage(String _from, String _to, BigInteger _sn, BigInteger _reqId); + void CallMessage(String _from, String _to, BigInteger _sn, BigInteger _reqId, byte[] _data); /** * Notifies that the call message has been executed. diff --git a/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceImpl.java b/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceImpl.java index 918e143e..a175979e 100644 --- a/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceImpl.java +++ b/xcall/src/main/java/foundation/icon/btp/xcall/CallServiceImpl.java @@ -31,6 +31,7 @@ import score.annotation.Payable; import java.math.BigInteger; +import java.util.Arrays; public class CallServiceImpl implements BSH, CallService, CallServiceEvent, FeeManage { public static final int MAX_DATA_SIZE = 2048; @@ -99,6 +100,10 @@ private void cleanupCallRequest(BigInteger sn) { requests.set(sn, null); } + private byte[] getDataHash(byte[] data) { + return Context.hash("keccak-256", data); + } + @Override @Payable @External @@ -141,9 +146,11 @@ public BigInteger sendCallMessage(String _to, byte[] _data, @Optional byte[] _ro @Override @External - public void executeCall(BigInteger _reqId) { + public void executeCall(BigInteger _reqId, byte[] _data) { CSMessageRequest req = proxyReqs.get(_reqId); Context.require(req != null, "InvalidRequestId"); + // compare the given data hash with the saved one + Context.require(Arrays.equals(getDataHash(_data), req.getData()), "DataHashMismatch"); // cleanup proxyReqs.set(_reqId, null); @@ -151,7 +158,7 @@ public void executeCall(BigInteger _reqId) { CSMessageResponse msgRes = null; try { DAppProxy proxy = new DAppProxy(Address.fromString(req.getTo())); - proxy.handleCallMessage(req.getFrom(), req.getData()); + proxy.handleCallMessage(req.getFrom(), _data); msgRes = new CSMessageResponse(req.getSn(), CSMessageResponse.SUCCESS, ""); } catch (UserRevertedException e) { int code = e.getCode(); @@ -201,7 +208,7 @@ public void executeRollback(BigInteger _sn) { @Override @EventLog(indexed=3) - public void CallMessage(String _from, String _to, BigInteger _sn, BigInteger _reqId) {} + public void CallMessage(String _from, String _to, BigInteger _sn, BigInteger _reqId, byte[] _data) {} @Override @EventLog(indexed=1) @@ -267,11 +274,12 @@ private void handleRequest(String netFrom, BigInteger sn, byte[] data) { String to = msgReq.getTo(); BigInteger reqId = getNextReqId(); - CSMessageRequest req = new CSMessageRequest(from.toString(), to, msgReq.getSn(), msgReq.needRollback(), msgReq.getData()); + CSMessageRequest req = new CSMessageRequest(from.toString(), to, msgReq.getSn(), msgReq.needRollback(), + getDataHash(msgReq.getData())); proxyReqs.set(reqId, req); // emit event to notify the user - CallMessage(from.toString(), to, msgReq.getSn(), reqId); + CallMessage(from.toString(), to, msgReq.getSn(), reqId, msgReq.getData()); } private void handleResponse(String netFrom, BigInteger sn, byte[] data) { diff --git a/xcall/src/test/java/foundation/icon/btp/xcall/CallServiceImplTest.java b/xcall/src/test/java/foundation/icon/btp/xcall/CallServiceImplTest.java index 4104901a..74edd9a7 100644 --- a/xcall/src/test/java/foundation/icon/btp/xcall/CallServiceImplTest.java +++ b/xcall/src/test/java/foundation/icon/btp/xcall/CallServiceImplTest.java @@ -148,6 +148,7 @@ void handleBTPMessageShouldEmitCallMessage() { assertEquals(to.account(), el.get_to()); assertEquals(srcSn, el.get_sn()); assertEquals(reqId, el.get_reqId()); + assertArrayEquals(data, el.get_data()); }); MockBMCIntegrationTest.mockBMC.handleBTPMessage( checker, csAddress, @@ -159,15 +160,22 @@ void handleBTPMessageShouldEmitCallMessage() { @Test void executeCallWithoutSuccessResponse() { var from = new BTPAddress(linkNet, sampleAddress.toString()); + byte[] data = requestMap.get(srcSn).getData(); + + // should fail if data is not the expected one + AssertRevertedException.assertUserReverted(0, () -> + callSvc.executeCall(reqId, "fakeData".getBytes()) + ); + var checker = CSIntegrationTest.messageReceivedEvent((el) -> { assertEquals(from.toString(), el.get_from()); - assertArrayEquals(requestMap.get(srcSn).getData(), el.get_data()); + assertArrayEquals(data, el.get_data()); }).andThen(CSIntegrationTest.callExecutedEvent((el) -> { assertEquals(reqId, el.get_reqId()); assertEquals(CSMessageResponse.SUCCESS, el.get_code()); assertEquals("", el.get_msg()); })).andThen(MockBMCIntegrationTest.sendMessageEventShouldNotExists()); - callSvc.executeCall(checker, reqId); + callSvc.executeCall(checker, reqId, data); } @Order(3) @@ -332,7 +340,7 @@ void executeCallWithFailureResponse() { assertEquals(CSMessageResponse.FAILURE, el.get_code()); assertEquals(response.getMsg(), el.get_msg()); })); - callSvc.executeCall(checker, reqId); + callSvc.executeCall(checker, reqId, data); } @Order(12) From 152fc93b2f5e4f1f5e5a8710a96b92bec2fad68d Mon Sep 17 00:00:00 2001 From: Jaechang Namgoong Date: Fri, 23 Jun 2023 14:16:31 +0900 Subject: [PATCH 2/2] Bump xcall version to 0.6.2 --- xcall/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xcall/build.gradle b/xcall/build.gradle index 5cadfb5e..67f5af3e 100644 --- a/xcall/build.gradle +++ b/xcall/build.gradle @@ -1,4 +1,4 @@ -version = '0.6.1' +version = '0.6.2' dependencies { compileOnly("foundation.icon:javaee-api:$javaeeVersion")