icon-project / icon-bridge

The centralized bridge of ICON
Apache License 2.0
21 stars 15 forks source link

Java SCORE review for existing Codebase and Testing #682

Open pragyanshrestha-ibriz opened 2 years ago

pragyanshrestha-ibriz commented 2 years ago

What needs to be done

Review and Test the existing codebase for the xcall service already existing in the BTP repo for feasibility and identify the update required

Why it needs to be done

Identify the usability of the codebase and code changes required.

Tests

Link associated tests here

Stories

Link associated stories here

Acceptance Criteria

Describe how we can know whether the task is done.

Ex:

pragyanshrestha-ibriz commented 2 years ago

@CyrusVorwald-ICON Current implementation of iconbridge contracts requires service handler to be registered on bmc contract. There is no provision to do such in case of xCall implementation. Since any contract can execute xCall's interface, this opens the possibility of an arbitrary contract from making calls to a costly destination (draining the relay fund in process). To avoid such DOS attack type problem, proper fee handling mechanism, one based on destination chain and/or routing involved, is required This routing dependent fee handling is what has recently been added to BMC contract and is a function of BMC alone and not bound to xCall's exposed methods.

If we choose to develop xCall without new bmc-fee-mechanism, there won't be changes to estimated xCall timeline. Else if we find it necessary, the timeline will have to be updated accordingly. Since the bmc-fee-mechanism is a recent addition and has not been documented, an updated timeline can only be inferred based on our understanding of what the full design could be.

Note: the problem is present in current implementation of iconbridge as well. Though blacklisting restrictions could be used as a safeguard, a user could use multiple different addresses to avoid blacklist.

CyrusVorwald commented 2 years ago

Please refer to https://docs.google.com/document/d/1LGf4ptrCF_F3l1Rsn3_TxrlhJcZBGB8mx7RwUFlHAQc/edit#heading=h.cq70qal3dyv for fee details for now.