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 2 commits
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
20 changes: 9 additions & 11 deletions evm/chains/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,20 +487,18 @@ def apply_transaction(self, transaction):
heavy and incurs significant perferomance overhead.
"""
vm = self.get_vm()
block, receipt, computation = vm.apply_transaction(transaction)
receipts = block.get_receipts(self.chaindb) + [receipt]
base_block = vm.block

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.chaindb.persist_trie_data_dict(tx_kv_nodes)
self.chaindb.persist_trie_data_dict(receipt_kv_nodes)
new_header, receipt, computation = vm.apply_transaction(transaction)

self.header = block.header.copy(
transaction_root=tx_root_hash,
receipt_root=receipt_root_hash,
)
transactions = base_block.transactions + (transaction, )
receipts = base_block.get_receipts(self.chaindb) + (receipt, )

new_block = vm.seal_block(base_block, new_header, transactions, receipts)

self.header = new_block.header

return block.copy(header=self.header), receipt, computation
return new_block, receipt, computation

def estimate_gas(self, transaction, at_header=None):
if at_header is None:
Expand Down
2 changes: 1 addition & 1 deletion evm/db/chain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,7 @@ def get_block_uncles(self, uncles_hash: bytes) -> List[BlockHeader]:
#
# Transaction and Receipt API
#
@to_list
@to_tuple
def get_receipts(self, header: BlockHeader, receipt_class: Type[Receipt]) -> Iterable[Receipt]:
receipt_db = HexaryTrie(db=self.db, root_hash=header.receipt_root)
for receipt_idx in itertools.count():
Expand Down
28 changes: 16 additions & 12 deletions evm/vm/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,8 @@ def apply_transaction(self, transaction):
gas_used=receipt.gas_used,
state_root=state_root,
)
self.block = self.block.copy(
header=new_header,
transactions=tuple(self.block.transactions) + (transaction,),
)

return self.block, receipt, computation
return new_header, receipt, computation

#
# Mining
Expand Down Expand Up @@ -181,25 +177,33 @@ def import_block(self, block):
in block.transactions
]
if execution_data:
_, receipts, _ = zip(*execution_data)
headers, receipts, _ = zip(*execution_data)
header_with_txns = headers[-1]
else:
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)

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(
header=self.block.header.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: 2 additions & 3 deletions tests/core/chain-object/test_chain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,10 @@ def test_import_block_validation(valid_chain, funded_address, funded_address_ini


def test_import_block(chain, tx):
vm = chain.get_vm()
*_, computation = vm.apply_transaction(tx)
new_block, _, computation = chain.apply_transaction(tx)
assert computation.is_success

block = chain.import_block(vm.block)
block = chain.import_block(new_block)
assert block.transactions == (tx,)
assert chain.get_block_by_hash(block.hash) == block
assert chain.get_canonical_block_by_number(block.number) == block
Expand Down
17 changes: 7 additions & 10 deletions tests/core/vm/test_vm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,20 @@ def test_apply_transaction(
funded_address_private_key,
funded_address_initial_balance):
vm = chain.get_vm()
tx_idx = len(vm.block.transactions)
recipient = decode_hex('0xa94f5374fce5edbc8e2a8697c15331677e6ebf0c')
amount = 100
from_ = funded_address
tx = new_transaction(vm, from_, recipient, amount, funded_address_private_key)
*_, computation = vm.apply_transaction(tx)
new_header, _, computation = vm.apply_transaction(tx)

assert not computation.is_error
tx_gas = tx.gas_price * constants.GAS_TX
account_db = vm.state.account_db
assert account_db.get_balance(from_) == (
funded_address_initial_balance - amount - tx_gas)
assert account_db.get_balance(recipient) == amount
block = vm.block
assert block.transactions[tx_idx] == tx
assert block.header.gas_used == constants.GAS_TX

assert new_header.gas_used == constants.GAS_TX


def test_mine_block_issues_block_reward(chain):
Expand All @@ -46,14 +44,13 @@ def test_mine_block_issues_block_reward(chain):


def test_import_block(chain, funded_address, funded_address_private_key):
vm = chain.get_vm()
recipient = decode_hex('0xa94f5374fce5edbc8e2a8697c15331677e6ebf0c')
amount = 100
from_ = funded_address
tx = new_transaction(vm, from_, recipient, amount, funded_address_private_key)
*_, computation = vm.apply_transaction(tx)
tx = new_transaction(chain.get_vm(), from_, recipient, amount, funded_address_private_key)
new_block, _, computation = chain.apply_transaction(tx)

assert not computation.is_error
parent_vm = chain.get_chain_at_block_parent(vm.block).get_vm()
block = parent_vm.import_block(vm.block)
parent_vm = chain.get_chain_at_block_parent(new_block).get_vm()
block = parent_vm.import_block(new_block)
assert block.transactions == (tx,)
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