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

Statediff for full node #6

Merged
merged 9 commits into from
Feb 21, 2019
Merged

Statediff for full node #6

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3a5f7b7
Open a trie from the in-memory database
elizabethengelman Feb 5, 2019
498831e
Use a node's LeafKey as an identifier instead of the address
elizabethengelman Feb 7, 2019
79686c9
Make sure that statediff has been processed before pruning
elizabethengelman Feb 8, 2019
08d0fe3
Use blockchain stateCache.OpenTrie for storage diffs
elizabethengelman Feb 12, 2019
e380580
Clean up log lines and remove unnecessary fields from builder
elizabethengelman Feb 12, 2019
7995c5a
Apply go fmt changes
elizabethengelman Feb 13, 2019
4d6ddf9
Add a sleep to the blockchain test
elizabethengelman Feb 14, 2019
f2047cc
Address PR comments
elizabethengelman Feb 14, 2019
272d2f7
Address PR comments
elizabethengelman Feb 20, 2019
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
3 changes: 3 additions & 0 deletions statediff/testhelpers/mocks/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package mocks
import (
"errors"

"time"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
"github.com/ethereum/go-ethereum/core/types"
Expand Down Expand Up @@ -45,6 +47,7 @@ func (bc *BlockChain) SubscribeChainEvent(ch chan<- core.ChainEvent) event.Subsc
subscription := event.NewSubscription(func(quit <-chan struct{}) error {
for _, chainEvent := range bc.ChainEvents {
if eventCounter > 1 {
time.Sleep(250 * time.Millisecond)
Copy link
Author

@elizabethengelman elizabethengelman Feb 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I also wanted to note that I added a sleep in here, to get the service test suite passing, which isn't ideal. I have a suspicion that this is an indication that something else may be going on in the service loop, so @rmulhol or @i-norden if you have a moment to take another look at it, that would be awesome. Or, perhaps we can walk through it together - it's a lot to keep in one's head! 🤯

Copy link

Choose a reason for hiding this comment

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

Would definitely take you up on walking through it together - you've definitely got a better grasp of the larger project architecture than me!

That said I have no strong objection to including the sleep in a mock. My only question would be whether the need for a sleep here implies that actual asynchronous execution might run into a similar problem as whatever was causing the test to fail.

Copy link
Collaborator

@i-norden i-norden Feb 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I would also greatly appreciate a walk through but I was so slow to get back to this that you may have already done one! I don't think this would be what is causing the problem in the service loop, but while looking I noticed the break here and I've got myself confused about named breaks again. I'm under the impression this breaks out of the for loop entirely, not just the current select-case, so it not only skips the current block in the channel but stops listening altogether?

Copy link
Author

Choose a reason for hiding this comment

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

Just realizing that I forgot to mention that named breaks during the brief walk through. My impression is the same as yours @i-norden, where it would break out of the for loop entirely. My thought for including it in the break that you mentioned was that if a block didn't have a parent, then something wrong, and we should quit the process.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That makes sense and I agree we should quit the process. This is really nitpicky but the error kind of makes it sound like we are only skipping that block but otherwise continuing.

Copy link
Author

Choose a reason for hiding this comment

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

That's a great point. I missed this before I merged it into the statediffing branch 🤦🏻‍♀️, but I'll make a note on that PR so that this comment is addressed before we open a PR upstream!

return subErr
}
select {
Expand Down