Registry sketch and implementation for EVM chains #1

tolak · 2022-10-28T03:12:46Z

This PR introduces the Registry module, which will contain the registration of

EVM chain registration (implemented in this PR)
Substrate chain registration (upcoming PR)
Bridge registration (upcoming PR)
Dex registration (upcoming PR)

tolak · 2022-11-04T06:41:19Z

Convert this PR to draft, will be opened again when finished refactor

tenheadedlion

LGTM, there are 2 points I want to discuss:

Is there a workaround to keep the current functionalities but remove the ink dependencies from the index module? the existence of ink libraries lessens the feel of SDK in index/.
abi.json should be placed in the application layer(the contracts/)

For the first point, I think it's possible to get rid of SpreadLayout and PackedLayout

// somewhere in index/
// don't care about SpreadLayout, which may not make it to ink4.0, introducing it breaks the abstraction
pub struct EvmChain {
    info: ChainInfo,
    assets: Vec<AssetInfo>
}

// somewhere in contracts/
#[ink(storage)]
pub struct SomeStorage {
   map: Mapping<String, Vec<u8>> // or [u8; 1024]
}

// in a contract method:
fn register_method(args: ...) {
  // create an EvmChain
 let chain = EvmChain::from(args...);
 self.map.insert(args.name, 
  chain.encode() // or: somehow, if `Vec<u8>` is not allowed as the value type of the mapping, think of a way to do this: `chain.to_array()`
);
}

tenheadedlion · 2022-11-07T16:00:02Z

index/Cargo.toml

@@ -15,6 +18,9 @@ hex = { version = "0.4.3", default-features = false }
 pink-extension = { version = "0.1", default-features = false, features = ["ink-as-dependency"] }
 pink-web3 = { git = "https://github.com/Phala-Network/pink-web3.git", branch = "pink", default-features = false, features = ["pink", "signing"]}
 xcm = { git = "https://github.com/paritytech/polkadot", branch = "release-v0.9.29", default-features = false }
+ink_storage = { version = "3", default-features = false }


In a previous PR I deliberately remove these ink! dependencies, is there a way we can do without them?

I totally understand, however, due to the layout mechanism of Ink storage, If we'd like to store our types in storage, we must declare types with layout derived. Like this issue talked about. There is why ink_storage was introduced, sure we can save everything in the contract as bytes, but that's also bad for the code readability. Ink4 seems abandon the layout, haven't gotten a certain answer from the team.

I don't see why storing things in bytes in contracts is bad for readability, with a pair of proper encoder and decoder there is no problem with readability. Actually ink! is the cause of unreadability, there is no need for ink to stay in index/, if you don't have time, I can take care of it in another PR

tenheadedlion · 2022-11-07T16:01:38Z

index/src/registry/res/erc20-abi.json

@@ -0,0 +1,156 @@
+[


Can we move this abi.json into the contracts/ directory?

If index contains stuff to interact with EVM contract, it's ok to put it here. Why you said it should belong to the application layer?

let's say it's just a matter of aesthetics, if index is playing the role of SDK, or Engine, then it shouldn't configure itself inside, it should provide an interface for users to configure it. If something should be hardcoded then it should be placed somewhere like constants.rs or abis.rs, anything that exists in the form of an asset(like png, txt) is suspiciously not a part of the code and should be placed outside, as the input to the application.

tenheadedlion

There are disagreements on styles and readability, let's find another time to talk about these things

tenheadedlion · 2022-11-07T17:28:14Z

index/src/registry/evm_chain.rs

+            transport,
+            // PHA address
+            token_address,
+            include_bytes!("./res/erc20-abi.json"),


my two cents objection to hardcoding: It seems like EvmChain can be initiated from abi_json:

impl EvmChain { fn new( abi_json: &[u8], ...) -> Result<Self> }

EvmChain is intended to be initiated in the contract layer, you can pass the abi.json to its constructor in some contract method.

or you can let EvmChain carries the abi_json, which is absolutely not a good idea because abi is used once and once only.

pub struct EvmChain { pub info: ChainInfo, pub assets: Vec<AssetInfo>, abi: Vec<u8> }

tenheadedlion · 2022-11-08T04:36:42Z

index/Cargo.toml

@@ -15,6 +18,9 @@ hex = { version = "0.4.3", default-features = false }
 pink-extension = { version = "0.1", default-features = false, features = ["ink-as-dependency"] }
 pink-web3 = { git = "https://github.com/Phala-Network/pink-web3.git", branch = "pink", default-features = false, features = ["pink", "signing"]}
 xcm = { git = "https://github.com/paritytech/polkadot", branch = "release-v0.9.29", default-features = false }
+ink_storage = { version = "3", default-features = false }


I don't see why storing things in bytes in contracts is bad for readability, with a pair of proper encoder and decoder there is no problem with readability. Actually ink! is the cause of unreadability, there is no need for ink to stay in index/, if you don't have time, I can take care of it in another PR

tenheadedlion · 2022-11-08T04:44:33Z

index/src/registry/res/erc20-abi.json

@@ -0,0 +1,156 @@
+[


let's say it's just a matter of aesthetics, if index is playing the role of SDK, or Engine, then it shouldn't configure itself inside, it should provide an interface for users to configure it. If something should be hardcoded then it should be placed somewhere like constants.rs or abis.rs, anything that exists in the form of an asset(like png, txt) is suspiciously not a part of the code and should be placed outside, as the input to the application.

tolak requested review from h4x3rotab and tenheadedlion October 28, 2022 03:17

tolak force-pushed the registry/evm-chain branch from 91e2d25 to 1994ce5 Compare November 2, 2022 12:17

tolak added 12 commits November 3, 2022 18:13

registry:evm-chain initial commit

d19b92d

registry:evm-chain impl Inspector

027334a

registry:evm-chain update inspector impl & test

c3d1e2e

registry:evm-chain compare event count

27656a3

remove evm signer impl

9339dca

update traits

dba16a8

impl AssetRegistry for evm chain

73e8b53

support set endpoint

db19749

WIP: fetch erc20 balance

dc8cb8b

fetch erc20 balance

5c56c04

rebase main

b290d3b

fix clippy

2a8b1e5

tolak force-pushed the registry/evm-chain branch from 1994ce5 to 2a8b1e5 Compare November 3, 2022 11:24

tolak added 2 commits November 3, 2022 20:11

fix test

9411978

fmt code

72639ca

tolak marked this pull request as draft November 4, 2022 06:40

tolak added 2 commits November 7, 2022 17:11

WIP: refactor code

14d84e2

refactor code

fdc83cf

tolak marked this pull request as ready for review November 7, 2022 13:48

tolak changed the title ~~Evm chain basic functions~~ Registry sketch and implementation for EVM chains Nov 7, 2022

tenheadedlion reviewed Nov 7, 2022

View reviewed changes

tenheadedlion self-requested a review November 8, 2022 05:01

tenheadedlion approved these changes Nov 8, 2022

View reviewed changes

tolak merged commit 38f0326 into main Nov 8, 2022

tolak deleted the registry/evm-chain branch November 8, 2022 05:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Registry sketch and implementation for EVM chains #1

Registry sketch and implementation for EVM chains #1

Uh oh!

tolak commented Oct 28, 2022 •

edited

Loading

Uh oh!

tolak commented Nov 4, 2022

Uh oh!

tenheadedlion left a comment •

edited

Loading

Uh oh!

tenheadedlion Nov 7, 2022

Uh oh!

tolak Nov 8, 2022

Uh oh!

tenheadedlion Nov 8, 2022

Uh oh!

tenheadedlion Nov 7, 2022

Uh oh!

tolak Nov 8, 2022

Uh oh!

tenheadedlion Nov 8, 2022

Uh oh!

tenheadedlion left a comment

Uh oh!

tenheadedlion Nov 7, 2022

Uh oh!

tenheadedlion Nov 8, 2022

Uh oh!

tenheadedlion Nov 8, 2022

Uh oh!

Uh oh!

Registry sketch and implementation for EVM chains #1

Registry sketch and implementation for EVM chains #1

Uh oh!

Conversation

tolak commented Oct 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tolak commented Nov 4, 2022

Uh oh!

tenheadedlion left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tenheadedlion left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tolak commented Oct 28, 2022 •

edited

Loading

tenheadedlion left a comment •

edited

Loading