[wip] Add failing test case to spot an accountsDb corruption bug #7143
Conversation
|
|
||
| current_slot += 1; | ||
| accounts.store(current_slot, &[(&pubkey, &zero_lamport_account)]); | ||
| for _ in 0..33000 { |
There was a problem hiding this comment.
If I reduce this number, this test passes...
| let accounts = reconstruct_accounts_db_via_serialization(accounts, current_slot); | ||
|
|
||
| error!("does fail due to a reconstruction bug:"); | ||
| assert_load_account(&accounts, current_slot, pubkey, zero_lamport); |
There was a problem hiding this comment.
This should behave exactly same as this identical assertion.
| error!("doesn't fail:"); | ||
| assert_load_account(&accounts, current_slot, pubkey, zero_lamport); | ||
|
|
||
| purge_zero_lamport_accounts(&accounts, current_slot); |
There was a problem hiding this comment.
If I remove this line, this test passes.
|
(@ryoqun - just a minor suggestion: it would be helpful if this PR title was a bit more descriptive. "Add failing test case to spot a bug": what bug? Perhaps "Add failing test case to spot an accountsDb corruption bug") |
ryoqun
changed the title
Exactly agreeable. :) Rename done. |
|
Closing as this is merged as part of another PR: https://github.com/solana-labs/solana/pull/7190/files#diff-2099c5256db4eb5975c8834af38f6456R1944 |
Related to #7117.
If this is a real bug, this is scary. Basically, this corrupts the very integrity of AccountsDB.
The problem here is that a StorageEntry with a zero-lamport account data is prematurely/incorrectly flushed, and the account becomes non-zero balance again, namely the older version of account is restored, resulting in unexpected internal hash error.
(I've been concerned with these problems in my mind since the first draft of #7013, which resorted to bizarre solution because of this, this indeed seems to be a real issue, now)