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.

Sign up for GitHub

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

Keep cooked pubkeys in cache #3122

Merged
merged 1 commit into from
Nov 25, 2021
Merged

Keep cooked pubkeys in cache #3122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions beacon_chain/beacon_chain_db.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type

# immutableValidatorsDb only stores the total count; it's a proxy for SQL
# queries.
immutableValidatorsDb*: DbSeq[ImmutableValidatorData2]
immutableValidatorsDb*: DbSeq[ImmutableValidatorDataDb2]
immutableValidators*: seq[ImmutableValidatorData2]

checkpoint*: proc() {.gcsafe, raises: [Defect].}
Expand Down Expand Up @@ -257,13 +257,13 @@ proc get*[T](s: DbSeq[T], idx: int64): T =
let found = queryRes.expectDb()
if not found: panic()

proc loadImmutableValidators(vals: DbSeq[ImmutableValidatorData]): seq[ImmutableValidatorData] =
proc loadImmutableValidators(vals: DbSeq[ImmutableValidatorDataDb2]): seq[ImmutableValidatorData2] =
result = newSeqOfCap[ImmutableValidatorData2](vals.len())
for i in 0 ..< vals.len:
result.add vals.get(i)

proc loadImmutableValidators(vals: DbSeq[ImmutableValidatorData2]): seq[ImmutableValidatorData2] =
for i in 0 ..< vals.len:
result.add vals.get(i)
let tmp = vals.get(i)
result.add ImmutableValidatorData2(
pubkey: tmp.pubkey.loadValid(),
withdrawal_credentials: tmp.withdrawal_credentials)

proc new*(T: type BeaconChainDB,
dir: string,
Expand Down Expand Up @@ -299,7 +299,7 @@ proc new*(T: type BeaconChainDB,
genesisDepositsSeq =
DbSeq[DepositData].init(db, "genesis_deposits").expectDb()
immutableValidatorsDb =
DbSeq[ImmutableValidatorData2].init(db, "immutable_validators2").expectDb()
DbSeq[ImmutableValidatorDataDb2].init(db, "immutable_validators2").expectDb()

# V1 - expected-to-be small rows get without rowid optimizations
keyValues = kvStore db.openKvStore("key_values", true).expectDb()
Expand All @@ -325,7 +325,7 @@ proc new*(T: type BeaconChainDB,
len = immutableValidatorsDb1.len()
while immutableValidatorsDb.len() < immutableValidatorsDb1.len():
let val = immutableValidatorsDb1.get(immutableValidatorsDb.len())
immutableValidatorsDb.add(ImmutableValidatorData2(
immutableValidatorsDb.add(ImmutableValidatorDataDb2(
pubkey: val.pubkey.loadValid().toUncompressed(),
withdrawal_credentials: val.withdrawal_credentials
))
Expand Down Expand Up @@ -505,7 +505,9 @@ proc updateImmutableValidators*(
while db.immutableValidators.len() < numValidators:
let immutableValidator =
getImmutableValidatorData(validators[db.immutableValidators.len()])
db.immutableValidatorsDb.add immutableValidator
db.immutableValidatorsDb.add ImmutableValidatorDataDb2(
pubkey: immutableValidator.pubkey.toUncompressed(),
withdrawal_credentials: immutableValidator.withdrawal_credentials)
db.immutableValidators.add immutableValidator

template toBeaconStateNoImmutableValidators(state: phase0.BeaconState):
Expand Down Expand Up @@ -657,7 +659,7 @@ proc getStateOnlyMutableValidators(

assign(
dstValidator.pubkey,
immutableValidators[i].pubkey.loadValid().toPubKey())
immutableValidators[i].pubkey.toPubKey())
assign(
dstValidator.withdrawal_credentials,
immutableValidators[i].withdrawal_credentials)
Expand Down
10 changes: 7 additions & 3 deletions beacon_chain/spec/datatypes/base.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,14 @@ type

# Non-spec type that represents the immutable part of a validator - an
# uncompressed key serialization is used to speed up loading from database
ImmutableValidatorData2* = object
ImmutableValidatorDataDb2* = object
pubkey*: UncompressedPubKey
withdrawal_credentials*: Eth2Digest

ImmutableValidatorData2* = object
pubkey*: CookedPubKey
withdrawal_credentials*: Eth2Digest
Copy link
Contributor

Choose a reason for hiding this comment

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

The types are hard to distinguish in actual code. Maybe rename ImmutableValidatorCookedData?

Copy link
Member Author

Choose a reason for hiding this comment

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

well, they follow one pattern (variable and type), this would simply be another, equally confusing name - ie either you focus on the cooked:ness, or the db:ness, don't see a clear advantage to either and this is the minimal patch

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sure that @zah has some advanced framework-based feature up his sleeve to deal with this that would resolve this 2-line problem with a multilayered abstract solution :)


# https://github.com/ethereum/consensus-specs/blob/v1.1.5/specs/phase0/beacon-chain.md#validator
Validator* = object
pubkey*: ValidatorPubKey
Expand Down Expand Up @@ -520,7 +524,7 @@ func getImmutableValidatorData*(validator: Validator): ImmutableValidatorData2 =
doAssert cookedKey.isSome,
"Cannot parse validator key: " & toHex(validator.pubkey)
ImmutableValidatorData2(
pubkey: cookedKey.get().toUncompressed(),
pubkey: cookedKey.get(),
withdrawal_credentials: validator.withdrawal_credentials)

# TODO when https://github.com/nim-lang/Nim/issues/14440 lands in Status's Nim,
Expand Down Expand Up @@ -875,7 +879,7 @@ proc load*(
if validators.lenu64() <= index.uint64:
none(CookedPubKey)
else:
some(validators[index.int].pubkey.loadValid())
some(validators[index.int].pubkey)

template hash*(header: BeaconBlockHeader): Hash =
hash(header.state_root)
Expand Down
2 changes: 1 addition & 1 deletion beacon_chain/statediff.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func applyValidatorIdentities(
hl: auto) =
for item in hl:
if not validators.add Validator(
pubkey: item.pubkey.loadValid().toPubKey(),
pubkey: item.pubkey.toPubKey(),
withdrawal_credentials: item.withdrawal_credentials):
raiseAssert "cannot readd"

Expand Down