Introduced IncrementalStake, for all Eras #2538
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
TimSheard
commented
Nov 3, 2021
TimSheard
commented
Nov 3, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
eras/shelley-ma/impl/src/Cardano/Ledger/ShelleyMA/Rules/Utxo.hs
Outdated
Show resolved
Hide resolved
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
JaredCorduan
reviewed
Nov 4, 2021
There was a problem hiding this comment.
@TimSheard overall this is really awesome!! I made some minor comments.
TimSheard
force-pushed
the
jc/incremental-stake-distribution
branch
from
e81788c
to
fc9c609
Compare
lehins
reviewed
Nov 4, 2021
lehins
approved these changes
Nov 4, 2021
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Rules/TestChain.hs
Outdated
Show resolved
Hide resolved
eras/shelley-ma/impl/src/Cardano/Ledger/ShelleyMA/Rules/Utxo.hs
Outdated
Show resolved
Hide resolved
TimSheard
force-pushed
the
jc/incremental-stake-distribution
branch
from
8b7c7d0
to
81abe93
Compare
JaredCorduan
approved these changes
Nov 5, 2021
TimSheard
force-pushed
the
jc/incremental-stake-distribution
branch
from
81abe93
to
6900bf2
Compare
nc6
requested changes
Nov 17, 2021
JaredCorduan
force-pushed
the
jc/incremental-stake-distribution
branch
from
c3ef302
to
c37360d
Compare
eras/shelley/impl/src/Cardano/Ledger/Shelley/API/ByronTranslation.hs
Outdated
Show resolved
Hide resolved
JaredCorduan
force-pushed
the
jc/incremental-stake-distribution
branch
from
c37360d
to
a107301
Compare
nc6
force-pushed
the
jc/incremental-stake-distribution
branch
from
a107301
to
bb7e7eb
Compare
|
@nc6, this is green from benchmarking standpoint. |
nc6
approved these changes
Dec 6, 2021
eras/shelley-ma/impl/src/Cardano/Ledger/ShelleyMA/Rules/Utxo.hs
Outdated
Show resolved
Hide resolved
| -- and combining them with the ordinary stake. Zero out the Ptr indexed stake in the result. | ||
| -- This function is meant to only be called at the Epoch boundary in the Snap Rule. It is not meant | ||
| -- to be used to transform the IncrementalStake. | ||
| resolveIncrementalPtrs :: Map Ptr (Credential 'Staking (Crypto era)) -> IncrementalStake (Crypto era) -> IncrementalStake (Crypto era) |
There was a problem hiding this comment.
Is there a reason in this case for returning IncrementalStake rather than simply Map Credential Coin? I assume we zero out the incremental stuff at the epoch boundary anyway?
There was a problem hiding this comment.
Alternatively, just move this function inside of incrementalStakeDistr, which is the only thing to use it anyway.
| -- the _utxo. This is aways safe to do, but if the _utxo field is big, this can be very expensive, | ||
| -- which defeats the purpose of memoizing the _stakeDistro field. So use of this function should be | ||
| -- restricted to tests and initializations, where the invariant should be maintained. | ||
| smartUTxOState :: |
There was a problem hiding this comment.
Better name? Not that I have any good ideas...
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Address/Bootstrap.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Address/Bootstrap.hs
Show resolved
Hide resolved
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/BenchmarkFunctions.hs
Outdated
Show resolved
Hide resolved
eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Examples/Consensus.hs
Outdated
Show resolved
Hide resolved
TimSheard
force-pushed
the
jc/incremental-stake-distribution
branch
from
bb7e7eb
to
3e387a3
Compare
Incrementally maintains the raw stake distributiona as well as the UTxO
TimSheard
force-pushed
the
jc/incremental-stake-distribution
branch
from
3e387a3
to
247e7f0
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
incrementally maintains the raw stake distribution as well as the UTxO.
Adds InrementalStake field to UTxOState
Utxo rule (Utxos rule in Alonzo) and the Snap rule are modified to use this new state
Added a property test that computes both ways at every Epoch Boundary.