Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

The storage isn't cleared after the destruction of the contract #95

Closed
n0cte opened this issue Sep 6, 2021 · 9 comments
Closed

The storage isn't cleared after the destruction of the contract #95

n0cte opened this issue Sep 6, 2021 · 9 comments

Comments

@n0cte
Copy link
Contributor

n0cte commented Sep 6, 2021

  • Branch: empty_data_encoder
  • Test: "get storage after selfdestruct" ​
@levlite
Copy link

levlite commented Sep 6, 2021

Hi Ilnure we need more details around this bug, and also what are you working on that you ran into this? Please provide more info, in Russian if need be I can have it translated, thank you

@n0cte
Copy link
Contributor Author

n0cte commented Sep 6, 2021

I needed to add a check for function RetrieveStorageAtByAddressAndStorageSlotAndBlockHash in the case when storage is empty. In the integration tests for the GLDToken contract, I added a destroy function for the contract. After that i started comparing storage data before destoying and after.

Geth:
before: [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 54 53 201 173 197 222 160 0 0]
after: [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 0 0 0 0]

ipld-eth-server:
before: [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 54 53 201 173 197 222 160 0 0]
after: [0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 54 53 201 173 197 222 160 0 0]

i think, ipld-eth-server should return zero values after contract destoying

@AFDudley
Copy link
Contributor

AFDudley commented Sep 6, 2021

We are a little in the weeds for me, but isn't there a flag we're using someplace else to address this? Or put another way, I suspect that the current release of the stack marks that data correctly it's just the mark isn't making it all the way to whatever you're accessing.

@i-norden
Copy link
Collaborator

i-norden commented Sep 6, 2021

When a state or storage node is destroyed post EIP-158 an entry at that path/leaf_key is made in the database that is flagged with the "Removed" node type to signify as such.

This issue I think makes an assumption that this is not occurring, but we do not know that the issue is at this level yet and in fact the unit test coverage of statediffing geth suggests this is not where the issue lies.

What is most likely occurring is that "Removed" node entries are in the database but are not being properly considered when querying the database.

Please see:
#94 (comment)
and
#94 (comment)

@levlite
Copy link

levlite commented Sep 7, 2021

@ramilexe can you please translate Ian's message above and make sure it is conveyed to Ilnur?

@i-norden
Copy link
Collaborator

i-norden commented Sep 7, 2021

Copying over from slack:

So the issue is probably with these stored functions https://github.com/vulcanize/ipld-eth-server/blob/master/db/migrations/00014_create_stored_functions.sql#L4 and https://github.com/vulcanize/ipld-eth-server/blob/master/db/migrations/00014_create_stored_functions.sql#L22

Or in how they are being used here and the other related pg queries below it https://github.com/vulcanize/ipld-eth-server/blob/master/pkg/eth/ipld_retriever.go#L118

If its not at the ipld-eth-server level it is at the indexer level, but not the builder level. There are unit tests for these edge cases for the statediff builder but I cant recall if there are for the indexer (although it isn't really an edge case at that level, since it's the same db model being indexed in the same capacity/fashion).

@n0cte
Copy link
Contributor Author

n0cte commented Sep 10, 2021

I tried running sql scripts (for block 54 in my environment):

Query:

SELECT storage_cids.cid, data, block_number, was_storage_removed(storage_path, block_number, '0x64e75b19d9ed7b06213b57d69d1ca03e5164071c33016620c8f9e9f9861cce62') AS removed
FROM eth.storage_cids
  INNER JOIN eth.state_cids ON (storage_cids.state_id = state_cids.id)
  INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
  INNER JOIN public.blocks ON (storage_cids.mh_key = blocks.key)
WHERE state_leaf_key = '0x3336da5dab0fa31eb4f9e1408dcacdb7bc0fc5da9837d1629be1010049e56dd8'
  AND storage_leaf_key = '0x405787fa12a823e0f2b7631cc41b3ba8828b3321ca811111fa75cd3aa3bb5ace'
  AND block_number <= (SELECT block_number FROM eth.header_cids WHERE block_hash = '0x64e75b19d9ed7b06213b57d69d1ca03e5164071c33016620c8f9e9f9861cce62')
  AND header_cids.id = (SELECT canonical_header_id(block_number))
  ORDER BY block_number DESC

Result:

cid | data | block_number | removed
bagmacgzasfla7wzuessejihdtrxqd5lqxc57egukbbricizz2ssrltex4uvq | 7KAwV4f6Eqgj4PK3YxzEGzuogoszIcqBERH6dc06o7tazoqJNjXJrcXeoAAA | 53 | false

Query:

SELECT state_cids.cid, data, block_number, was_state_removed(state_path, block_number, '0x64e75b19d9ed7b06213b57d69d1ca03e5164071c33016620c8f9e9f9861cce62') AS removed
FROM eth.state_cids
  INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
  INNER JOIN public.blocks ON (state_cids.mh_key = blocks.key)
WHERE state_leaf_key = '0x3336da5dab0fa31eb4f9e1408dcacdb7bc0fc5da9837d1629be1010049e56dd8'
  AND block_number <= 54
ORDER BY block_number DESC

Result:

cid | data | block_number | removed
baglacgzayxjemamg64rtzet6pwznzrydydsqbnstzkbcoo337lmaixmfurya | (EMPTY) | 54 | false
baglacgzatlspm5do4dernn2fwdmb42d2u67snrvhsaviarxbkeenxikfeyjq | -GmgIDbaXasPox60-eFAjcrNt7wPxdqYN9Fim-EBAEnlbdi4RvhEAYCgWTyxXtWK9XGnhbGQ8s-BRv7G6p_T2uIOEc3otVN9b8ugSFMHT4v_2dHaXBDzcd0PHJYgo92KC0xVQyynWsH9-1k | 53 | true

I think both scripts are working not correctly. Function was_storage_removed return not correct result. Second sql with LIMIT 1 also return bad result in function was_state_removed

I used two versions of docker:

vulcanize/dapptools:v0.29.0-v1.10.8-statediff-0.0.26
vulcanize/statediff-migrations:v0.6.0
vulcanize/dapptools:v0.29.0-v1.10.2-statediff-0.0.19
vulcanize/statediff-migrations:v0.4.0

@i-norden
Copy link
Collaborator

i-norden commented Sep 13, 2021

Thanks @n0cte, I think @arijitAD has fixed the state query issue in #99 but it's not clear what the issue is for storage.

I should have noticed this sooner, but the reason we needed to use these was_x_removed functions was because, prior to cerc-io/go-ethereum#58, we did not have the leaf key present for "Removed" nodes and so we would not find them using the basic query against leaf_key. Now that we have leaf_key present, these functions are unnecessary (and indeed broken in this new context).

E.g. instead of

SELECT state_cids.cid, data, was_state_removed(state_path, block_number, $2) AS removed
FROM eth.state_cids
    INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
    INNER JOIN public.blocks ON (state_cids.mh_key = blocks.key)
WHERE state_leaf_key = $1
AND block_number <= (SELECT block_number
    FROM eth.header_cids
    WHERE block_hash = $2)
AND header_cids.id = (SELECT canonical_header_id(block_number))
ORDER BY block_number DESC
LIMIT 1

We can just do

SELECT state_cids.cid, data, node_type
FROM eth.state_cids
    INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
    INNER JOIN public.blocks ON (state_cids.mh_key = blocks.key)
WHERE state_leaf_key = $1
AND block_number <= (SELECT block_number
    FROM eth.header_cids
    WHERE block_hash = $2)
AND header_cids.id = (SELECT canonical_header_id(block_number))
ORDER BY block_number DESC
LIMIT 1

This will return the latest node entry for a specific leaf key, including "Removed" types. We can check if the node_type returned is "Removed" and if it is handle the zero value appropriately. Alternatively, if we want to get the latest state of a node at a leaf_key (last value before it was "Removed"), we just need to add a condition to the query to exclude "Removed" node_types.

I.e.

SELECT state_cids.cid, data
FROM eth.state_cids
    INNER JOIN eth.header_cids ON (state_cids.header_id = header_cids.id)
    INNER JOIN public.blocks ON (state_cids.mh_key = blocks.key)
WHERE state_leaf_key = $1
AND block_number <= (SELECT block_number
    FROM eth.header_cids
    WHERE block_hash = $2)
AND header_cids.id = (SELECT canonical_header_id(block_number))
AND node_type IS NOT 3
ORDER BY block_number DESC
LIMIT 1

@arijitAD
Copy link
Contributor

Fixed this in #98

@arijitAD arijitAD reopened this Sep 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants