Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shard class and ShardVM class #256

Closed
hwwhww opened this issue Jan 9, 2018 · 6 comments
Closed

Shard class and ShardVM class #256

hwwhww opened this issue Jan 9, 2018 · 6 comments

Comments

@hwwhww
Copy link
Contributor

hwwhww commented Jan 9, 2018

What is wrong?

Create a Shard class which mirrors the Chain class and a ShardVM which mirrors the VM class.

How can it be fixed

Fill this in if you know how to fix it.

@hwwhww
Copy link
Contributor Author

hwwhww commented Jan 9, 2018

@pipermerriam I copied it from #238.

@hwwhww hwwhww added the eth2.0 label Jan 9, 2018
@pipermerriam
Copy link
Member

By the way, conceptually, I think this is the right way to go, but I think we'll have to play with the actual implementation some to figure out what the right way to do this is. Initially, my thoughts are:

  • BaseChain class with common/shared methods between Shard and Chain classes.
  • rename BaseVM to VM
  • new BaseVM class with common/shared methods between VM and ShardVM.

Also, we may not need a ShardVM class at all assuming that there is nothing special about the VM execution within shards (which I think is the case but not 100% sure).

@hwwhww
Copy link
Contributor Author

hwwhww commented Jan 9, 2018

Yes, agree! And it should be well-discussed and planned before we start too.

We can plan for archival nodes first. Some primary differences between VM and ShardVM may be:

  1. Different data structures between block and collation
    1. Different configure_header functions
    2. Some methods look reusable, we need to decide which of them need to be rewritten.
      • For example, block.parent_hash and collation.parent_collation_hash are really alike but also meaningful. (I think it’s for distinguishing with period_start_prevhash).
    3. Shard chain uses main chain’s Blockchain opcodes- blockhash, timestamp, block_number, difficulty, and gas_limit
      • I noted in Refactored VM, VMState, and Block + new apply_transaction #247 TODOs: Decouple VMState.block_header the VMState properties (blockhash, timestamp, block_number, difficulty, and gas_limit), because shard chains use period_start_prevhash block properties of main chain in contracts, but VMState.state_db() should use the state_root of VMState.collation_header to build the state root trie.
      • It can be modified in VM to keep the codes consistent but will be unnecessary for main chain.
  2. No PoW for shard
    1. No uncle and nephew rewards
    2. No compute_difficulty, only simple score increment. (collation_number ==collation score)

Also, I think we can benefit from drawing the whole JSON-RPC transaction sequence diagram between shard client and main chain. It could help us to figure out when/where should we call for the data from main chain.

@hwwhww
Copy link
Contributor Author

hwwhww commented Jan 9, 2018

@pipermerriam
By the way, do you think we should (1) arrange the common method in master branch and then implement it in sharding branch or (2) start with sharding branch and then backport
common classes later?

@hwwhww
Copy link
Contributor Author

hwwhww commented Jan 26, 2018

@pipermerriam

New BaseVM class with common/shared methods between VM and ShardVM.

I’m looking into this, it seems easy to drag out some functions that related to gas_limit and uncles to be VM-only. But on the other side, so many functions are using block as their prefix, suffix, or are related to block_class and I’m not sure which of them could be reused in ShardVM at this moment.

I suggest that we can start with a simple overriding at the beginning, and then
implement the new logic with collation in Stage 2:

  1. In base.py, rename VM to BaseVM.

  2. Create vm.py, VM, simply inherit BaseVM:

    class VM(BaseVM):
        pass
  3. Create shard_vm.py, also inherit BaseVM:

    class ShardVM(BaseVM):
        pass
  4. Rewire the ShardingVM fork to inherit this new ShardVM:

    ShardingVM = ShardVM.configure(
        name='ShardingVM',
        # classes
        _block_class=ShardingBlock,
        _state_class=ShardingVMState,
        # implementations from other VM forks
        create_header_from_parent=staticmethod(create_frontier_header_from_parent),
        compute_difficulty=staticmethod(compute_frontier_difficulty),
        configure_header=configure_frontier_header,
        ....
    )
  5. When we start to apply Collation as the container, we override the basic functions in ShardVM if we need.

  6. Refactor and clean up the common/shared methods.

I guess it would be less pain of backporting by this way. Any advice and suggestions will be greatly appreciated.

@pipermerriam
Copy link
Member

Approach looks sound. Make it so!

628b747f8ccdfb757062f36a27eedecfc2295f515c0586e05fbfb0620c0571a2

@hwwhww hwwhww changed the title STUB: Shard class and ShardVM class Shard class and ShardVM class Jan 26, 2018
@hwwhww hwwhww closed this as completed Jan 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants