Write state diff to CSV #2
@ -50,12 +50,12 @@ func (sdb *builder) BuildStateDiff(oldStateRoot, newStateRoot common.Hash, block
|
||||
|
|
||||
// Generate tries for old and new states
|
||||
oldTrie, err := trie.New(oldStateRoot, sdb.trieDB)
|
||||
if err != nil {
|
||||
log.Debug("error creating oldTrie", err)
|
||||
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error creating trie for oldStateRoot", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
newTrie, err := trie.New(newStateRoot, sdb.trieDB)
|
||||
if err != nil {
|
||||
log.Debug("error creating newTrie", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error creating trie for newStateRoot", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@ -64,7 +64,7 @@ func (sdb *builder) BuildStateDiff(oldStateRoot, newStateRoot common.Hash, block
|
||||
|
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 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 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 may be missing something but it looks like the 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 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
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. 🎉
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
newIt := newTrie.NodeIterator([]byte{})
|
||||
creations, err := sdb.collectDiffNodes(oldIt, newIt)
|
||||
if err != nil {
|
||||
log.Debug("error collecting creation diff nodes", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error collecting creation diff nodes", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@ -73,7 +73,7 @@ func (sdb *builder) BuildStateDiff(oldStateRoot, newStateRoot common.Hash, block
|
||||
|
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 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 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 may be missing something but it looks like the 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 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
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. 🎉
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
newIt = newTrie.NodeIterator(make([]byte, 0))
|
||||
deletions, err := sdb.collectDiffNodes(newIt, oldIt)
|
||||
if err != nil {
|
||||
log.Debug("error collecting deletion diff nodes", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error collecting deletion diff nodes", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@ -85,17 +85,17 @@ func (sdb *builder) BuildStateDiff(oldStateRoot, newStateRoot common.Hash, block
|
||||
|
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 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 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 may be missing something but it looks like the 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 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
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. 🎉
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
// Build and return the statediff
|
||||
updatedAccounts, err := sdb.buildDiffIncremental(creations, deletions, updatedKeys)
|
||||
if err != nil {
|
||||
log.Debug("error building diff incremental for updated", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error building diff for updated accounts", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
createdAccounts, err := sdb.buildDiffEventual(creations, true)
|
||||
if err != nil {
|
||||
log.Debug("error building diff incremental for created", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error building diff for created accounts", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
deletedAccounts, err := sdb.buildDiffEventual(deletions, false)
|
||||
if err != nil {
|
||||
log.Debug("error building diff incremental for deleted", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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.Error("Error building diff for deleted accounts", "error", err)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@ -260,9 +260,7 @@ func (sdb *builder) buildStorageDiffsIncremental(oldSR common.Hash, newSR common
|
||||
|
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 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 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 may be missing something but it looks like the 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 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
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. 🎉
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
}
|
||||
|
||||
func (sdb *builder) addressByPath(path []byte) (*common.Address, error) {
|
||||
// db := core.PreimageTable(sdb.chainDb)
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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("Looking up address from path", "path", hexutil.Encode(append([]byte("secure-key-"), path...)))
|
||||
// if addrBytes,err := db.Get(path); err != nil {
|
||||
|
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 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 may be missing something but it looks like the 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
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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 addrBytes, err := sdb.chainDB.Get(append([]byte("secure-key-"), hexToKeybytes(path)...)); err != nil {
|
||||
log.Error("Error looking up address via path", "path", hexutil.Encode(append([]byte("secure-key-"), path...)), "error", err)
|
||||
return nil, err
|
||||
|
||||
|
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 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 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 may be missing something but it looks like the 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 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
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. 🎉
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. 🎉
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
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 The difference between whether we'e tracking down deletions vs creations is in the order that we're passing the iterators to 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?
|
||||
@ -13,6 +13,7 @@ import (
|
||||
"github.com/ethereum/go-ethereum/common"
|
||||
"github.com/ethereum/go-ethereum/core/types"
|
||||
"github.com/ethereum/go-ethereum/event"
|
||||
"github.com/ethereum/go-ethereum/log"
|
||||
)
|
||||
|
||||
type BlockChain interface {
|
||||
@ -51,19 +52,30 @@ func (StateDiffService) APIs() []rpc.API {
|
||||
func (sds *StateDiffService) Loop(events chan core.ChainEvent) {
|
||||
for elem := range events {
|
||||
currentBlock := elem.Block
|
||||
|
||||
parentHash := currentBlock.ParentHash()
|
||||
parentBlock := sds.BlockChain.GetBlockByHash(parentHash)
|
||||
|
||||
sds.Extractor.ExtractStateDiff(*parentBlock, *currentBlock)
|
||||
stateDiffLocation, err := sds.Extractor.ExtractStateDiff(*parentBlock, *currentBlock)
|
||||
if err != nil {
|
||||
log.Error("Error extracting statediff", "block number", currentBlock.Number(), "error", err)
|
||||
} else {
|
||||
log.Info("Statediff extracted", "block number", currentBlock.Number(), "location", stateDiffLocation)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var eventsChannel chan core.ChainEvent
|
||||
func (sds *StateDiffService) Start(server *p2p.Server) error {
|
||||
events := make(chan core.ChainEvent, 10)
|
||||
sds.BlockChain.SubscribeChainEvent(events)
|
||||
go sds.Loop(events)
|
||||
log.Info("Starting statediff service")
|
||||
eventsChannel := make(chan core.ChainEvent, 10)
|
||||
sds.BlockChain.SubscribeChainEvent(eventsChannel)
|
||||
go sds.Loop(eventsChannel)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (StateDiffService) Stop() error { return nil }
|
||||
func (StateDiffService) Stop() error {
|
||||
log.Info("Stopping statediff service")
|
||||
close(eventsChannel)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
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
NodeIteratorinitialized withmake([]byte, 0)enables us to track down deletions while initializing it withmake([]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
NodeIteratorinitialized withmake([]byte, 0)enables us to track down deletions while initializing it withmake([]byte{})yields creations.I may be missing something but it looks like the
creationarg is unused (and I think that's true all the way up to the calls ofbuildDiffEventual), so we may be able to remove itI may be missing something but it looks like the
creationarg is unused (and I think that's true all the way up to the calls ofbuildDiffEventual), so we may be able to remove itYep, 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. 🎉
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?