search

darwinia-network / darwinia-docs-deprecated

Darwinia Documentation
https://docs.darwinia.network
20 stars 28 forks source link

Update: Interoperate design between substrate and dvm #176

Closed hackfisher closed 2 years ago

hackfisher commented 3 years ago

  1. If smart contracts want to call some substrate dispatch call, it should use Dispatch Precompile. Some refactor is required in mapping token factory implementations @xiaoch05 The can help remove the Issuing Precompile.

  2. For the dispatch call scale encode required by smart contract before call Dispatch Precompile, there are no currently solidity SCALE library, another useful approach is to use a similar way by providing an Util Precompile to help encode dispatch calls. @AsceticBear @xiaoch05

  3. The reserved Precompile addresses are limited resources, so we should encourage to group similar functions into Precompiles as actions. For example, we should merge Withdraw and KTON into one Precompile( Currency?), and for the new added dispatch call encoding function, it should be only partial actions of Utils Precompile, and Utils Precompile can be extended to support more functions in future. @AsceticBear

  4. Security concerns related to infinite loop between smart contracts and dispatch call, with Dispatch Precompile, any ethereum address, including smart contract will be able to call any dispatch call as soon as it support normal Origin. So if some dispatch call support to pass in a callback in form of calling a user contract's method, then some attacker can build such a contract by call this dispatch call with a callback back to the contract's method it self, and causing infinite loop.

Need to test whether such an infinite loop is really a problem for us. @AsceticBear

So we should add a security practice that we should not add user contract's method as callback of a dispatch call, or We must be very carefully to audit it for the reasons mentioned above. (H. in https://github.com/darwinia-network/darwinia/wiki/Best-Practice-for-Darwinia-Security-Review)

boundless-forest commented 3 years ago

For example, we should merge Withdraw and KTON into one Precompile( Currency?)

I want to discuss more this thing. Withdraw and Kton precompile appear to be doing similar things, they all are responsible for transferring/withdrawing Darwinia native token to dvm. Now, we have already deployed Withdraw precompile in Pangolin and Crab. Withdraw precompile only in Pangolin. In case of merging these two precompile into one, we might beak the protocol of Kton and Withdraw precompile design, Something that comes to my mind:

  1. Change the current input value and add a module selector to distinguish.
  2. Change Kton precompile address or change WithDraw precompile address.

The current implementation can lead to more possibilities for combinations if we keep it. It's would be cheaper to disable or enable one precompile and make the logic of these two critical precompiles more clear. In fact, they did two different things, two precompiles, and two native tokens. I suggest just keep this two precompile separated like before. In regards to the new precompile for dispatch calls, Utils Precompile would be better.

hackfisher commented 3 years ago

For example, we should merge Withdraw and KTON into one Precompile( Currency?)

I want to discuss more this thing. Withdraw and Kton precompile appear to be doing similar things, they all are responsible for transferring/withdrawing Darwinia native token to dvm. Now, we have already deployed Withdraw precompile in Pangolin and Crab. Withdraw precompile only in Pangolin. In case of merging these two precompile into one, we might beak the protocol of Kton and Withdraw precompile design, Something that comes to my mind:

  1. Change the current input value and add a module selector to distinguish.
  2. Change Kton precompile address or change WithDraw precompile address.

The current implementation can lead to more possibilities for combinations if we keep it. It's would be cheaper to disable or enable one precompile and make the logic of these two critical precompiles more clear. In fact, they did two different things, two precompiles, and two native tokens. I suggest just keep this two precompile separated like before. In regards to the new precompile for dispatch calls, Utils Precompile would be better.

The only issue I saw is compatibility, If only Withdraw Precompile is on Crab, why not just remove KTON Precompile and add KTON functions to Withdraw and rename to Currency (address not changed).

For Pangolin, since it is a testnet, compatibility issue is acceptable and can be ignored.

boundless-forest commented 2 years ago

I look over the points listed above, I think we can close this issue now. @HackFisher @xiaoch05