Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Incorrect tx receipt trie root #231

Merged
merged 22 commits into from Feb 20, 2019
Merged

Conversation

nicholasjpaterno
Copy link
Contributor

@nicholasjpaterno nicholasjpaterno commented Nov 21, 2018

Neither the transactionsTrie nor the receiptsTrie were being calculated and the resulting transactionsRoot and receiptsRoot were always identical (EMPTY_TRIE_ROOT) when there were 1 or more transactions in a block.

This PR suggests a fix by:
Creating 2 empty Tries and populating them with tx index (within the block) as the keys and transactions/receipts for their respective values. The resulting Trie's roots are then copied to block.header.transactionsTrie and block.header.receiptTrie respectively.

Copy link
Member

@davidmurdoch davidmurdoch left a comment

Choose a reason for hiding this comment

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

I'd prefer not to see test refactor changes in these commits (I know we've done these drive-by refactorings in the past) since chris is currently refactoring everything to async/await right now. There may end up being merge conflicts with #313.

lib/blockchain_double.js Outdated Show resolved Hide resolved
test/block_tags.js Outdated Show resolved Hide resolved
test/block_tags.js Outdated Show resolved Hide resolved
@nicholasjpaterno
Copy link
Contributor Author

I'd prefer not to see test refactor changes in these commits (I know we've done these drive-by refactorings in the past) since chris is currently refactoring everything to async/await right now. There may end up being merge conflicts with #313.

Agreed, just for background this test was updated before any refactoring started. I did make sure to notify and remind chris of the changes to hopefully avoid any conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants