-
Notifications
You must be signed in to change notification settings - Fork 156
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
Fold the keyDeposit into the UMap #3217
Conversation
8abf077
to
6378a36
Compare
c916bad
to
c80a94a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me, thank you Tim! I made a few comments, but only very minor stuff.
e39b82e
to
625f1ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Few suggestions, some more important than others, but nothing that is blocking this PR.
eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState/IncrementalStake.hs
Outdated
Show resolved
Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState/IncrementalStake.hs
Outdated
Show resolved
Hide resolved
eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState/RefundsAndDeposits.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Generator/Trace/Chain.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/test/Test/Cardano/Ledger/Shelley/Examples/Combinators.hs
Outdated
Show resolved
Hide resolved
libs/cardano-ledger-binary/testlib/Test/Cardano/Ledger/Binary/TreeDiff.hs
Outdated
Show resolved
Hide resolved
7233960
to
6c180a7
Compare
Data.UMap(UMap). It will store a Word64 instead of a Coin, and its interface uses (CompactForm Coin) which is a newtype around a Word64. Defined the dsUnified field of DState to be this new UMap. Updated all uses to this new interface. Changed UMapCompact to roll key deposits into the Unified map. Removed dsDeposits in DState, fixed code to use new UMap version of deposit map. Changed DState CBOR, golden encoding, added changelog entries
6c180a7
to
039c042
Compare
We change the Rewards View of the UMap so that it map to a Reward-Deposit-Pair, so the single view stores both
the reward amount (a (CompactForm Coin)) for a staking credential, and a deposit amount (a (CompactForm Coin))
for the same credential.
Because we have UMap version of deposits, we no longer need the dsDeposits field for DState. It was removed.
The purpose of this PR is to save space, as there are millions of staking credentials, Since we are using compact coins, this should take about 1 extra Word64 per credential for deposits, and 2 less Word64 per credential for rewards (which used to be Coin). So this actually uses less memory.
A few things in the tools Cardano.Ledger.State.Query will need to fixed in a follow on PR