Write state diff to CSV #2

Merged
elizabethengelman merged 47 commits from ee-state-diff into statediff-for-archive-node 2019-01-28 21:31:02 +00:00
3 changed files with 20 additions and 27 deletions
Showing only changes of commit 96caa16268 - Show all commits

View File

@ -162,7 +162,7 @@ func (sdb *builder) buildDiffEventual(accounts map[common.Address]*state.Account
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
if created { if created {
nonce := DiffUint64{ Value: &val.Nonce } nonce := DiffUint64{ Value: &val.Nonce }
balance := DiffBigInt{ Value: val.Balance } balance := DiffBigInt{ Value: val.Balance }
contractRoot := DiffString{ NewValue: &hexRoot } contractRoot := DiffString{ Value: &hexRoot }
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
accountDiffs[addr] = AccountDiffEventual{ accountDiffs[addr] = AccountDiffEventual{
Nonce: nonce, Nonce: nonce,
Balance: balance, Balance: balance,
@ -174,7 +174,7 @@ func (sdb *builder) buildDiffEventual(accounts map[common.Address]*state.Account
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
} else { } else {
nonce := DiffUint64{ Value: &val.Nonce } nonce := DiffUint64{ Value: &val.Nonce }
balance := DiffBigInt{ Value: val.Balance } balance := DiffBigInt{ Value: val.Balance }
contractRoot := DiffString{ OldValue: &hexRoot } contractRoot := DiffString{ Value: &hexRoot }
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
accountDiffs[addr] = AccountDiffEventual{ accountDiffs[addr] = AccountDiffEventual{
Nonce: nonce, Nonce: nonce,
Balance: balance, Balance: balance,
@ -204,7 +204,7 @@ func (sdb *builder) buildDiffIncremental(creations map[common.Address]*state.Acc
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
codeHash := hexutil.Encode(createdAcc.CodeHash) codeHash := hexutil.Encode(createdAcc.CodeHash)
nHexRoot := createdAcc.Root.Hex() nHexRoot := createdAcc.Root.Hex()
contractRoot := DiffString{ NewValue: &nHexRoot } contractRoot := DiffString{ Value: &nHexRoot }
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
updatedAccounts[common.HexToAddress(val)] = AccountDiffIncremental{ updatedAccounts[common.HexToAddress(val)] = AccountDiffIncremental{
Nonce: nonce, Nonce: nonce,
@ -234,11 +234,7 @@ func (sdb *builder) buildStorageDiffsEventual(sr common.Hash, creation bool) (ma
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
log.Debug("Found leaf in storage", "path", pathToStr(it)) log.Debug("Found leaf in storage", "path", pathToStr(it))
path := pathToStr(it) path := pathToStr(it)
value := hexutil.Encode(it.LeafBlob()) value := hexutil.Encode(it.LeafBlob())
if creation { storageDiffs[path] = DiffString{Value: &value}
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
storageDiffs[path] = DiffString{NewValue: &value}
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
} else {
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
storageDiffs[path] = DiffString{OldValue: &value}
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
}
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
} }
cont := it.Next(true) cont := it.Next(true)
if !cont { if !cont {
@ -268,7 +264,7 @@ func (sdb *builder) buildStorageDiffsIncremental(oldSR common.Hash, newSR common
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
log.Debug("Found leaf in storage", "path", pathToStr(it)) log.Debug("Found leaf in storage", "path", pathToStr(it))
path := pathToStr(it) path := pathToStr(it)
value := hexutil.Encode(it.LeafBlob()) value := hexutil.Encode(it.LeafBlob())
storageDiffs[path] = DiffString{NewValue: &value} storageDiffs[path] = DiffString{Value: &value}
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
} }
cont := it.Next(true) cont := it.Next(true)

rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:02:54 +00:00 (Migrated from github.com)
Review

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a NodeIterator initialized with make([]byte, 0) enables us to track down deletions while initializing it with make([]byte{}) yields creations.

I'm wondering if there's a way to use variable naming to help reveal what's happening here. I might just be being dense but I'm having trouble understanding why a `NodeIterator` initialized with `make([]byte, 0)` enables us to track down deletions while initializing it with `make([]byte{})` yields creations.
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
rmulhol commented 2019-01-03 22:54:49 +00:00 (Migrated from github.com)
Review

I may be missing something but it looks like the creation arg is unused (and I think that's true all the way up to the calls of buildDiffEventual), so we may be able to remove it

I may be missing something but it looks like the `creation` arg is unused (and I think that's true all the way up to the calls of `buildDiffEventual`), so we may be able to remove it
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:16:50 +00:00 (Migrated from github.com)
Review

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉

Yep, nice catch. Previously we were storing the old value and the new value of each diff, and whether the account was created/deleted affected which value was used as the old. But since removing the old values, we shouldn't need that bool anymore. 🎉
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?
elizabethengelman commented 2019-01-09 15:55:05 +00:00 (Migrated from github.com)
Review

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between make(byte[], 0) and []byte{} was inadvertent, so I'll switch them both to be the same for consistency.

The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to collectDiffNodes - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations).

I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

That's a great point, this is confusing. The gist of it is that we're creating iterators from the old and new tries, and when we're creating the iterators we're passing in a starting key, which I think can just be 0. The difference between `make(byte[], 0)` and `[]byte{}` was inadvertent, so I'll switch them both to be the same for consistency. The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to `collectDiffNodes` - where we're either finding nodes in the old trie that aren't in the new trie (deletions) or nodes in the new trie that we're in the old (creations). I feel like there's a way to make this a bit more readable, but it hasn't come to me. Do you have any ideas?

View File

@ -116,7 +116,7 @@ func TestBuilder(t *testing.T) {
Balance: b.DiffBigInt{ Value: big.NewInt(balanceChange10000) }, Balance: b.DiffBigInt{ Value: big.NewInt(balanceChange10000) },
Code: nil, Code: nil,
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot, OldValue: nil }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
burnAddress: { burnAddress: {
@ -124,7 +124,7 @@ func TestBuilder(t *testing.T) {
Balance: b.DiffBigInt{ Value: big.NewInt(miningReward)}, Balance: b.DiffBigInt{ Value: big.NewInt(miningReward)},
Code: nil, Code: nil,
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot, OldValue: nil }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
}, },
@ -134,7 +134,7 @@ func TestBuilder(t *testing.T) {
Nonce: b.DiffUint64{ Value: &nonce1 }, Nonce: b.DiffUint64{ Value: &nonce1 },
Balance: b.DiffBigInt{ Value: big.NewInt(testBankFunds.Int64() - balanceChange10000) }, Balance: b.DiffBigInt{ Value: big.NewInt(testBankFunds.Int64() - balanceChange10000) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
}, },
@ -159,7 +159,7 @@ func TestBuilder(t *testing.T) {
Balance: b.DiffBigInt{ Value: big.NewInt(balanceChange1000) }, Balance: b.DiffBigInt{ Value: big.NewInt(balanceChange1000) },
Code: nil, Code: nil,
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot}, ContractRoot: b.DiffString{ Value: &originalContractRoot},
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
contractAddr: { contractAddr: {
@ -167,7 +167,7 @@ func TestBuilder(t *testing.T) {
Balance: b.DiffBigInt{ Value: big.NewInt(0) }, Balance: b.DiffBigInt{ Value: big.NewInt(0) },
Code: []byte{96, 96, 96, 64, 82, 96, 0, 53, 124, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 144, 4, 128, 99, 96, 205, 38, 133, 20, 96, 65, 87, 128, 99, 193, 100, 49, 185, 20, 96, 107, 87, 96, 63, 86, 91, 0, 91, 96, 85, 96, 4, 128, 128, 53, 144, 96, 32, 1, 144, 145, 144, 80, 80, 96, 169, 86, 91, 96, 64, 81, 128, 130, 129, 82, 96, 32, 1, 145, 80, 80, 96, 64, 81, 128, 145, 3, 144, 243, 91, 96, 136, 96, 4, 128, 128, 53, 144, 96, 32, 1, 144, 145, 144, 128, 53, 144, 96, 32, 1, 144, 145, 144, 80, 80, 96, 138, 86, 91, 0, 91, 128, 96, 0, 96, 0, 80, 131, 96, 100, 129, 16, 21, 96, 2, 87, 144, 144, 1, 96, 0, 91, 80, 129, 144, 85, 80, 91, 80, 80, 86, 91, 96, 0, 96, 0, 96, 0, 80, 130, 96, 100, 129, 16, 21, 96, 2, 87, 144, 144, 1, 96, 0, 91, 80, 84, 144, 80, 96, 199, 86, 91, 145, 144, 80, 86}, Code: []byte{96, 96, 96, 64, 82, 96, 0, 53, 124, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 144, 4, 128, 99, 96, 205, 38, 133, 20, 96, 65, 87, 128, 99, 193, 100, 49, 185, 20, 96, 107, 87, 96, 63, 86, 91, 0, 91, 96, 85, 96, 4, 128, 128, 53, 144, 96, 32, 1, 144, 145, 144, 80, 80, 96, 169, 86, 91, 96, 64, 81, 128, 130, 129, 82, 96, 32, 1, 145, 80, 80, 96, 64, 81, 128, 145, 3, 144, 243, 91, 96, 136, 96, 4, 128, 128, 53, 144, 96, 32, 1, 144, 145, 144, 128, 53, 144, 96, 32, 1, 144, 145, 144, 80, 80, 96, 138, 86, 91, 0, 91, 128, 96, 0, 96, 0, 80, 131, 96, 100, 129, 16, 21, 96, 2, 87, 144, 144, 1, 96, 0, 91, 80, 129, 144, 85, 80, 91, 80, 80, 86, 91, 96, 0, 96, 0, 96, 0, 80, 130, 96, 100, 129, 16, 21, 96, 2, 87, 144, 144, 1, 96, 0, 91, 80, 84, 144, 80, 96, 199, 86, 91, 145, 144, 80, 86},
CodeHash: "0x1c671ee4ae8abbacab7da59d6f8785cce8295eb086551ce7ac266a2e93666c0f", CodeHash: "0x1c671ee4ae8abbacab7da59d6f8785cce8295eb086551ce7ac266a2e93666c0f",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot}, ContractRoot: b.DiffString{ Value: &originalContractRoot},
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
}, },
@ -177,21 +177,21 @@ func TestBuilder(t *testing.T) {
Nonce: b.DiffUint64{ Value: &nonce2 }, Nonce: b.DiffUint64{ Value: &nonce2 },
Balance: b.DiffBigInt{ Value: big.NewInt(block1BankBalance - balanceChange1000) }, Balance: b.DiffBigInt{ Value: big.NewInt(block1BankBalance - balanceChange1000) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
account1Addr: { account1Addr: {
Nonce: b.DiffUint64{ Value: &nonce2 }, Nonce: b.DiffUint64{ Value: &nonce2 },
Balance: b.DiffBigInt{ Value: big.NewInt(block1Account1Balance - balanceChange1000 + balanceChange1000) }, Balance: b.DiffBigInt{ Value: big.NewInt(block1Account1Balance - balanceChange1000 + balanceChange1000) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
burnAddress: { burnAddress: {
Nonce: b.DiffUint64{ Value: &nonce0 }, Nonce: b.DiffUint64{ Value: &nonce0 },
Balance: b.DiffBigInt{ Value: big.NewInt(miningReward + miningReward) }, Balance: b.DiffBigInt{ Value: big.NewInt(miningReward + miningReward) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
}, },
@ -217,24 +217,24 @@ func TestBuilder(t *testing.T) {
Nonce: b.DiffUint64{ Value: &nonce0 }, Nonce: b.DiffUint64{ Value: &nonce0 },
Balance: b.DiffBigInt{ Value: big.NewInt(block2Account2Balance + miningReward) }, Balance: b.DiffBigInt{ Value: big.NewInt(block2Account2Balance + miningReward) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
contractAddr: { contractAddr: {
Nonce: b.DiffUint64{ Value: &nonce1 }, Nonce: b.DiffUint64{ Value: &nonce1 },
Balance: b.DiffBigInt{ Value: big.NewInt(0) }, Balance: b.DiffBigInt{ Value: big.NewInt(0) },
CodeHash: "0x1c671ee4ae8abbacab7da59d6f8785cce8295eb086551ce7ac266a2e93666c0f", CodeHash: "0x1c671ee4ae8abbacab7da59d6f8785cce8295eb086551ce7ac266a2e93666c0f",
ContractRoot: b.DiffString{ NewValue: &newContractRoot }, ContractRoot: b.DiffString{ Value: &newContractRoot },
Storage: map[string]b.DiffString{ Storage: map[string]b.DiffString{
"0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace": { "0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace": {
NewValue: &newStorageValue }, Value: &newStorageValue },
}, },
}, },
testBankAddress: { testBankAddress: {
Nonce: b.DiffUint64{ Value: &nonce3 }, Nonce: b.DiffUint64{ Value: &nonce3 },
Balance: b.DiffBigInt{ Value: big.NewInt(99989000) }, Balance: b.DiffBigInt{ Value: big.NewInt(99989000) },
CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", CodeHash: "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
ContractRoot: b.DiffString{ NewValue: &originalContractRoot }, ContractRoot: b.DiffString{ Value: &originalContractRoot },
Storage: map[string]b.DiffString{}, Storage: map[string]b.DiffString{},
}, },
}, },

View File

@ -72,9 +72,6 @@ type AccountDiffIncremental struct {
Storage map[string]DiffString `json:"storage" gencodec:"required"` Storage map[string]DiffString `json:"storage" gencodec:"required"`
} }
type DiffString struct { type DiffString struct{ Value *string `json:"value" gencodec:"optional"` }
NewValue *string `json:"newValue" gencodec:"optional"` type DiffUint64 struct{ Value *uint64 `json:"value" gencodec:"optional"` }
OldValue *string `json:"oldValue" gencodec:"optional"` type DiffBigInt struct{ Value *big.Int `json:"value" gencodec:"optional"` }
}
type DiffUint64 struct { Value *uint64 `json:"value" gencodec:"optional"` }
type DiffBigInt struct { Value *big.Int `json:"value" gencodec:"optional"` }