[Draft] snapshot bank fields protection#8185
[Draft] snapshot bank fields protection#8185ryoqun wants to merge 1 commit intosolana-labs:masterfrom
Conversation
| epoch, | ||
| blockhash_queue: RwLock::new(parent.blockhash_queue.read().unwrap().clone()), | ||
|
|
||
| // TODO: clean this up, soo much special-case copying... |
There was a problem hiding this comment.
Phew, this initialization code block holds quite a lot fields. ;)
I've examined each fields for BankConfig or not; As this is still a draft, the categorization is rough and some might be wrong.
| &signature_count_buf, | ||
| self.last_blockhash().as_ref(), | ||
| bank_config_buf.as_ref(), // mainly for snapshot | ||
| status_cache_hash.as_ref(), // as par #7053 |
| // Its serialized binary is hashed into the `slot_hash` for the slot of a frozen bank. | ||
| // So, this struct will be hashed at every slot boundary, so this is preferred to be small | ||
| #[derive(Serialize)] | ||
| pub struct BankConfig { // or BankState // slot_config |
There was a problem hiding this comment.
Is the idea then that Bank is no longer serialized into snapshots? Just this new BankConfig struct?
There was a problem hiding this comment.
@mvines Yeah, ideally, it could be made so for the security separation perspective. However, I don't think that the serialization target change (Bank->BankConfig) is the scope of this PR at the moment. I want to minimize the required work here because of timing pressure. So, I want to just reuse some already-existing snapshot code as is, which is deeply rooted to the Bank while being secured enough (Like accounts db setup).
|
I like the direction! Only the network-wide relevant data is required to be in snapshots. Removing the local-only state should ease verification and security |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
stale
Bot
added
the
stale
|
This stale pull request has been automatically closed. Thank you for your contributions. |
stale
Bot
removed
the
stale
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
stale
Bot
added
the
stale
|
This stale pull request has been automatically closed. Thank you for your contributions. |
3 participants
Problem
struct Bankfields aren't protected when loaded from snapshots.For example, some fields like
capitalizationis only used at the epoch boundary. So, unless we cover the integrity check for those fields at snapshot restore, victim validator could divert from the cluster at the next epoch a lot later even if they did SPV when restoring from a third-party snapshot.Summary of Changes
Introduce
BankConfigwhich is hashed into the slot hash and SPV-ed (ref: #6936).And make child bank's all state be derived only from it.
This PR still is a draft; I may be wrong. :)
I want to gather feedback about my understanding for the current situation and above problem/solution making senses and general implementation directions. :)
Part of #7167