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

v1.0: Include account.owner into account hash #9920

Merged
merged 3 commits into from
May 7, 2020

Conversation

ryoqun
Copy link
Contributor

@ryoqun ryoqun commented May 7, 2020

manual backport of #9917 with gating logic for the v1.0.x / mainnet-beta

fixes #9916

@ryoqun
Copy link
Contributor Author

ryoqun commented May 7, 2020

@mvines Thanks for review!

I'm testing this locally now with mainnet-beta snapshots...

@@ -1060,13 +1069,20 @@ impl AccountsDB {
hasher.result()
}

pub fn include_owner_in_hash(slot: Slot) -> bool {
// Account hashing updated to include owner activates at this slot on the mainnet-beta
slot >= 10_000_000
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mvines Well, I'd like want a rounded number; but I think this should be 11_000_000. The reason is two-fold:

  1. Relatively small upgrade time window: 32 hours (*)
  2. This happens Friday night in PT

11_000_000 is around 5/13-5/14 around the world (*)

(*):

$ solana  --url http://api.mainnet-beta.solana.com epoch-info

Slot: 9705495
Epoch: 22
Epoch Slot Range: [9504000..9936000)
Epoch Completed Percent: 46.642%
Epoch Completed Slots: 201495/432000 (230505 remaining)
Epoch Completed Time: 22h 23m 18s/2days (1day 1h 36m 42s remaining)

$ pry
[1]  pry(main)> ((10_000_000 - 9705495) / 2.5) / 3600
=> 32.72277777777778
[6] pry(main)> Time.now + ((11_000_000 - 9705495) / 2.5)
=> 2020-05-14 03:37:39 +0900                                                           

Copy link
Member

Choose a reason for hiding this comment

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

👍

@mvines mvines changed the title Include account.owner into account hash v1.0: Include account.owner into account hash May 7, 2020
@mvines mvines added the automerge Merge this Pull Request automatically once CI passes label May 7, 2020
@ryoqun
Copy link
Contributor Author

ryoqun commented May 7, 2020

Ok, I've finished all testing for mainnet-beta. It works as intended. And fortunately we're still in consensus without any bit-corrupt/malicious traces. :)

I've compared two different snapshots from our trusted validators and I saw only the increase of normal accounts and there is no strange owner.

$ grep -E 'owner' accounts1.yml | sort | uniq -c
   2124   - owner: '11111111111111111111111111111111'
     20   - owner: 'Config1111111111111111111111111111111111111'
      4   - owner: 'NativeLoader1111111111111111111111111111111'
    566   - owner: 'Stake11111111111111111111111111111111111111'
      9   - owner: 'Sysvar1111111111111111111111111111111111111'
     93   - owner: 'Vote111111111111111111111111111111111111111'
$ grep -E 'owner' accounts2.yml | sort | uniq -c
   2127   - owner: '11111111111111111111111111111111'
     20   - owner: 'Config1111111111111111111111111111111111111'
      4   - owner: 'NativeLoader1111111111111111111111111111111'
    566   - owner: 'Stake11111111111111111111111111111111111111'
      9   - owner: 'Sysvar1111111111111111111111111111111111111'
     93   - owner: 'Vote111111111111111111111111111111111111111'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants