Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Sanitize zero lamport accounts in append vecs #9083

Merged
merged 1 commit into from
Mar 29, 2020

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented Mar 26, 2020

(this hampers TdS snapshot restore as it contain some now-illegalized account data; I'm planning to fix it against the live TdS cluster) (fixed that)

Problem

Snapshot can contain any state for 0 lamport accounts, which can be exposed to the runtime. And cause cluster forks like did #8590.

Summary of Changes

Like #8657, which normalized the account state when saving, this pr does sanitize the account state when restored from snapshot.

Part of #7167

@ryoqun ryoqun requested a review from sakridge March 26, 2020 09:05
@codecov
Copy link

codecov bot commented Mar 26, 2020

Codecov Report

Merging #9083 into master will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff           @@
##           master   #9083   +/-   ##
======================================
  Coverage    80.5%   80.5%           
======================================
  Files         269     269           
  Lines       58946   58970   +24     
======================================
+ Hits        47462   47484   +22     
- Misses      11484   11486    +2     

@mvines
Copy link
Contributor

mvines commented Mar 26, 2020

(this hampers TdS snapshot restore as it contain some now-illegalized account data; I'm planning to fix it against the live TdS cluster)

@ryoqun - did you observe a problem with the current TdS cluster here?

@ryoqun
Copy link
Contributor Author

ryoqun commented Mar 27, 2020

(this hampers TdS snapshot restore as it contain some now-illegalized account data; I'm planning to fix it against the live TdS cluster)

@ryoqun - did you observe a problem with the current TdS cluster here?

@mvines Yeah, that's because TdS cluster still holds ill-state zero lamport accounts before we had #8657.. I'm purging it by solana pay now..

@ryoqun
Copy link
Contributor Author

ryoqun commented Mar 27, 2020

The bad ones are like this (Note that rent_epoch...):

[runtime/src/append_vec.rs:99] ("bad", self.meta.pubkey, self.clone_account()) = (
    "bad",
    5pxapmiZ6CcePZU4cGVb6nHynhXzMXhDUBgvP9CPgpJJ,
    Account { lamports: 0 data.len: 0 owner: 11111111111111111111111111111111 executable: false rent_epoch: 18 hash: 11111111111111111111111111111111 },
)
[runtime/src/append_vec.rs:296] self = AppendVec {
    path: "/home/ryoqun/work/solana/solana/check4/accounts/2017204.7486295",
    map: MmapMut {
        ptr: 0x00007f4a7cc00000,
        len: 4194304,
    },  
    append_offset: Mutex {
        data: 191944,
    },  
    current_len: 191944,
    file_size: 4194304,
}   
[runtime/src/append_vec.rs:99] ("bad", self.meta.pubkey, self.clone_account()) = (
    "bad",
    3Z5XVczCTXeYeFABoeFm1LngC9657kZMVGNFzqFXviHb,
    Account { lamports: 0 data.len: 0 owner: 11111111111111111111111111111111 executable: false rent_epoch: 14 hash: 11111111111111111111111111111111 },
)   
[runtime/src/append_vec.rs:296] self = AppendVec {
    path: "/home/ryoqun/work/solana/solana/check4/accounts/290195.1111884",
    map: MmapMut {
        ptr: 0x00007f4a59400000,
        len: 4194304,
    },  
    append_offset: Mutex {
        data: 51318,
    },  
    current_len: 51318,
    file_size: 4194304,
}   

@ryoqun ryoqun marked this pull request as ready for review March 27, 2020 07:11
@ryoqun
Copy link
Contributor Author

ryoqun commented Mar 27, 2020

(this hampers TdS snapshot restore as it contain some now-illegalized account data; I'm planning to fix it against the live TdS cluster)

@ryoqun - did you observe a problem with the current TdS cluster here?

@mvines Yeah, that's because TdS cluster still holds ill-state zero lamport accounts before we had #8657.. I'm purging it by solana pay now..

Now, I've fixed all those!

@sakridge Prime time for review!

@ryoqun ryoqun changed the title [wip] Sanitize zero lamport accounts in append vecs Sanitize zero lamport accounts in append vecs Mar 27, 2020
Copy link
Contributor

@sakridge sakridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mvines mvines added the v1.1 label Mar 28, 2020
@ryoqun ryoqun merged commit 729cc4e into solana-labs:master Mar 29, 2020
mergify bot pushed a commit that referenced this pull request Mar 29, 2020
solana-grimes pushed a commit that referenced this pull request Mar 29, 2020

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
automerge
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants