Implement child chain and client #26
Conversation
Saves entire merkle instead of just root
DavidKnott
force-pushed
the
implement-child-chain
branch
from
9420568
to
0a3ce89
Compare
|
@jdkanani Glad you're working on Plasma MVP! Given that this OMG's plasma repo it'd be great if the focus stayed on reviewing this PR. |
|
@DavidKnott Thank you. Sorry about that. Removed my comment :) |
plasma/child_chain/child_chain.py
Outdated
| spent = self.blocks[blknum].transaction_set[txindex].spent2 | ||
| amount = self.blocks[blknum].transaction_set[txindex].amount2 | ||
| assert spent is False | ||
| spent = True |
There was a problem hiding this comment.
spent = True doesn't seem to have a purpose
plasma/child_chain/child_chain.py
Outdated
| self.current_block = Block() | ||
|
|
||
| def get_transaction(self, blknum, txindex): | ||
| return rlp.encode(self.block[blknum].transaction_set[txindex]).hex() |
There was a problem hiding this comment.
Should be self.blocks[...]
| @@ -2,22 +2,34 @@ | |||
|
|
|||
| We're implementing the [Minimum Viable Plasma](https://ethresear.ch/t/minimal-viable-plasma/426) | |||
|
|
|||
| Current repo includes: | |||
| Install dependencies with: | |||
There was a problem hiding this comment.
I think this generally needs to be cleaned up but that's for another PR.
| from plasma.child_chain.transaction import Transaction | ||
|
|
||
|
|
||
| @main.command('start') |
There was a problem hiding this comment.
Possibly not a great UX. omg start <command> is more annoying to use than omg <command>.
There was a problem hiding this comment.
@kfichter The user only has to enter omg start once which enters them into the cli and from there they just enter <command>.
There was a problem hiding this comment.
Ah okay, understood.
plasma/cli/client.py
Outdated
| self.client = Client() | ||
| self.db = plyvel.DB('/tmp/plasma_mvp_db/', create_if_missing=True) | ||
| self.current_block = self.client.get_current_block_num() | ||
| self.synced_block = 1 |
There was a problem hiding this comment.
synced_block and client_cmds could be converted to class attributes.
plasma/cli/main.py
Outdated
| ) | ||
|
|
||
|
|
||
| @click.group(invoke_without_command=True) |
There was a problem hiding this comment.
Needs to be @click.group(invoke_without_command=True, context_settings=CONTEXT_SETTINGS) for this to work.
plasma/client/child_chain_service.py
Outdated
| "jsonrpc": "2.0", | ||
| "id": 0, | ||
| } | ||
| response = requests.post(self.url, data=json.dumps(payload), |
There was a problem hiding this comment.
Easier to use requests.post(self.url, json=payload).json(). This also removes the need for the additional Content-Type header.
plasma/client/client.py
Outdated
| def __init__(self, root_chain_provider=HTTPProvider('http://localhost:8545'), child_chain_url="http://localhost:8546/jsonrpc"): | ||
| deployer = Deployer(root_chain_provider) | ||
| abi = json.load(open("contract_data/RootChain.json")) | ||
| self.w3 = deployer.w3 |
There was a problem hiding this comment.
Client.w3 is never used after this, could just replace with self.root_chain = deployer.w3.eth...
|
|
||
| def create_contract(self, path, args=(), gas=4410000, sender=t.k0): | ||
| abi, bytecode = self.compile_contract(path, args) | ||
| abi, bytecode, contract_name = self.compile_contract(path, args) |
There was a problem hiding this comment.
This change needs to be implemented here:
https://github.com/omisego/plasma-mvp/blob/30b7cfbd9e713f3093cda1cb2660f518fe523f0c/tests/conftest.py#L60
Can be implemented as
abi, bytecode, _ = Deployer().compile_contract(path, args)as we don't need the contract name in order to complete that test.
make test fails otherwise.
| 'ethereum==2.1.3', | ||
| 'web3', | ||
| 'ethereum==2.3.0', | ||
| 'web3==3.16.4', |
There was a problem hiding this comment.
Closes #18, conveniently
|
@kfichter I made some of the changes you suggested. |
plasma/cli/client.py
Outdated
| self.synced_block = 1 | ||
| self.client_cmds = dict( | ||
| synced_block = 1 | ||
| client_cmds = dict( |
There was a problem hiding this comment.
Moving client_cmds to a class variable means we don't have access to self so we need to change to:
client_cmds = dict(
sync=ClientParser.sync_child_chain,
deposit=ClientParser.deposit,
send_tx=ClientParser.send_tx,
submit_block=ClientParser.submit_block,
withdraw=ClientParser.withdraw,
help=ClientParser.help,
)
plasma/cli/client.py
Outdated
| def process_input(self, inp): | ||
| self.inp = inp | ||
| command = self.inp[0] | ||
| if command not in self.client_cmds: | ||
| if command not in client_cmds: |
There was a problem hiding this comment.
We still need the self reference unless we make client_cmds a global here.
plasma/client/child_chain_service.py
Outdated
| @@ -22,8 +22,7 @@ def send_request(self, method, args): | |||
| "jsonrpc": "2.0", | |||
| "id": 0, | |||
| } | |||
| response = requests.post(self.url, data=json.dumps(payload), | |||
| headers=headers).json() | |||
| response = requests.post(self.url, data=payload).json() | |||
There was a problem hiding this comment.
Should be json=payload if we want to send as JSON w/o headers = {'content-type': 'application/json'}. Might as well remove headers = {'content-type': 'application/json'} if we're going this route.
remove empty utxo
Implements the Plasma child chain and a cli to interface with both the plasma child chain and the root chain.
The next PRs will add more tests, contain validations for child chain transactions, and will further improve the README.
@paulperegud @robinclart