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.

Sign up for GitHub

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

Return header instead of block from vm.apply_txn #625

Merged
merged 4 commits into from
May 2, 2018
Merged

Return header instead of block from vm.apply_txn #625

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4cbec43
Return header instead of block from vm.apply_txn
carver May 2, 2018
811bb8a
Extract block sealing logic into method
carver May 2, 2018
57b1649
bugfix: make lint
carver May 2, 2018
9258805
rename seal_block -> set_block_transactions
carver May 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 5 additions & 16 deletions evm/chains/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,27 +487,16 @@ def apply_transaction(self, transaction):
heavy and incurs significant perferomance overhead.
"""
vm = self.get_vm()
old_block = vm.block
base_block = vm.block

new_header, receipt, computation = vm.apply_transaction(transaction)

transactions = old_block.transactions + (transaction, )
tx_root_hash, tx_kv_nodes = make_trie_root_and_nodes(transactions)
self.chaindb.persist_trie_data_dict(tx_kv_nodes)
transactions = base_block.transactions + (transaction, )
receipts = base_block.get_receipts(self.chaindb) + (receipt, )

receipts = old_block.get_receipts(self.chaindb) + (receipt, )
receipt_root_hash, receipt_kv_nodes = make_trie_root_and_nodes(receipts)
self.chaindb.persist_trie_data_dict(receipt_kv_nodes)
new_block = vm.seal_block(base_block, new_header, transactions, receipts)

self.header = new_header.copy(
transaction_root=tx_root_hash,
receipt_root=receipt_root_hash,
)

new_block = old_block.copy(
header=self.header,
transactions=transactions,
)
self.header = new_block.header

return new_block, receipt, computation

Expand Down
28 changes: 15 additions & 13 deletions evm/vm/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,30 +178,32 @@ def import_block(self, block):
]
if execution_data:
headers, receipts, _ = zip(*execution_data)
header_with_txns = headers[-1]
else:
headers, receipts = tuple(), tuple()
receipts = tuple()
header_with_txns = self.block.header

tx_root_hash, tx_kv_nodes = make_trie_root_and_nodes(block.transactions)
receipt_root_hash, receipt_kv_nodes = make_trie_root_and_nodes(receipts)

self.block = self.seal_block(self.block, header_with_txns, block.transactions, receipts)

return self.mine_block()

def seal_block(self, base_block, new_header, transactions, receipts):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like the term seal as for me at least, it implies mining like setting the mix_hash and nonce.

What about just add_transaction_to_block or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about set_transactions_in_block?

  • pro: transactions is plural, old transactions in block are replaced, and -- for now -- the trie data is persisted
  • con: implies block mutation, when it's really building a block copy

Other alternatives:

  • place_transactions_in_block
  • commit_transactions_to_block
  • get_block_with_transactions (problem: get implies no side-effects)
  • add_transactions_to_block (problem: add implies that previous transactions are left in place)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • set_block_transactions (keeps with get_, set_ convention...)
  • update_block_transactions just an alternate prefix.

any of of those work for you?


tx_root_hash, tx_kv_nodes = make_trie_root_and_nodes(transactions)
self.chaindb.persist_trie_data_dict(tx_kv_nodes)
self.chaindb.persist_trie_data_dict(receipt_kv_nodes)

if len(headers):
header_with_txns = headers[-1]
else:
header_with_txns = self.block.header
receipt_root_hash, receipt_kv_nodes = make_trie_root_and_nodes(receipts)
self.chaindb.persist_trie_data_dict(receipt_kv_nodes)

self.block = self.block.copy(
transactions=block.transactions,
header=header_with_txns.copy(
return base_block.copy(
transactions=transactions,
header=new_header.copy(
transaction_root=tx_root_hash,
receipt_root=receipt_root_hash,
),
)

return self.mine_block()

def mine_block(self, *args, **kwargs):
"""
Mine the current block. Proxies to self.pack_block method.
Expand Down
5 changes: 4 additions & 1 deletion tests/json-fixtures/test_state.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,10 @@ def test_state_fixtures(fixture, fixture_vm_class):
)

try:
block, _, computation = vm.apply_transaction(transaction)
header, receipt, computation = vm.apply_transaction(transaction)
transactions = vm.block.transactions + (transaction, )
receipts = vm.block.get_receipts(chaindb) + (receipt, )
block = vm.seal_block(vm.block, header, transactions, receipts)
except ValidationError as err:
block = vm.block
transaction_error = err
Expand Down