-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add function to check for state node removal. #19
Conversation
24f922c
to
754776b
Compare
754776b
to
d66bef1
Compare
WHERE state_leaf_key = key | ||
AND block_number <= (SELECT block_number FROM eth.header_cids WHERE block_hash = hash) | ||
ORDER BY state_cids.id DESC LIMIT 1 | ||
LOOP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this going to iterate over every entry for this leaf key below the provided block height (hash) before returning FALSE if the leaf was never removed below the provided height?
I think a more performant way to do this might be to simply check for a record with this state_leaf_key and node_type = 3 below the provided height, if it returns empty this contract was not deleted below the height and if such a record exists then it was (a contract can never be destroyed and then deployed to the same address/state_leaf_key, since the contract address is a function of the deploying account's address and the deploying account's nonce- which must increment each time).
Something like
SELECT exists(*) FROM eth.state_cids INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
WHERE state_leaf_key = key
AND block_number <= (SELECT block_number FROM eth.header_cids WHERE block_hash = hash)
AND node_type = 3
This type of query should, hopefully, be relatively fast now that we have a btree index on the node_type. Adding a ORDER BY block_number DESC
might speed it up too for the case where a REMOVED node does exist since the REMOVED node would have to be the latest entry and that would force the query planner to search in descending order whereas, I think, otherwise it may not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, an even better approach building on the train of thought at the end of the last comment: knowing that if a contract was destroyed there can never be entries for that leaf key again, we can search backwards from the provided height/hash and as soon as we find a node with that leaf key we can return TRUE if the node_type = 3 and FALSE if the node_type = 2.
So in a function we can do something like
SELECT node_type FROM eth.state_cids INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
WHERE state_leaf_key = key
AND block_number <= (SELECT block_number FROM eth.header_cids WHERE block_hash = hash)
ORDER BY block_number DESC
LIMIT 1
If the node_type we get is 3 we return TRUE, if its 2 (leaf node) we return FALSE.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And actually the knowledge about being unable to redeploy to the same address is irrelevant... even if we could redeploy, if the latest entry we see for a contract address below a height is REMOVED, then we know the contract is empty at the specified height (it would have to be empty until we saw a new LEAF entry (redeployment) at that address).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, and just FYI, I am wrong and it is actually possible to deploy a contract to the same address using the CREATE2
opcode since it uses some supplied salt parameter instead of the account nonce. But in any case, that's irrelevant here as mentioned above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.