Restrict Checkpoints to be constructible from genesis only
#3048
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
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/fromGenesis
branch
from
d6b8b31
to
941980f
Compare
Base automatically changed from
HeinrichApfelmus/ADP-1043/1288-reorganize-modules
to
master
December 10, 2021 10:14
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/fromGenesis
branch
2 times, most recently
from
78b88ca
to
e2d8e6c
Compare
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/fromGenesis
branch
from
e2d8e6c
to
c54becd
Compare
paweljakubas
approved these changes
Dec 15, 2021
There was a problem hiding this comment.
LGTM! Left several suggestions. Maybe you will find defining new sum error in putCheckpoint method of DBLayer interface reasonable. I feel we should not differentiate no wallet and no genesis from each other in a sense they are both very serious/critical and if so why not to treat them on the same level ;-)
The function `fromGenesis` replaces the `singleton` function.
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/fromGenesis
branch
from
163c67a
to
d68b1e5
Compare
|
bors merge |
iohk-bors bot
added a commit
that referenced
this pull request
Dec 16, 2021
3048: Restrict `Checkpoints` to be constructible from genesis only r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1043 ### Overview Previous work in epic ADP-1043 introduced delta encodings, DBVars, and an embedding of the wallet state and its delta encodings into a database table. It's time to integrate these tools with the wallet code. To facilitate code review, the integration proceeds in a sequence of refactorings that do not change functionality and pass all unit tests. In this step, we implement a suggestion that came up during review where we remove the `singleton` function in favor of `fromGenesis`, making sure that the `Checkpoints` structure always contains a checkpoint for the genesis block. ### Details * The `initializeWallet` function will now throw an exception if it is incorrectly called with a checkpoint that is not at the genesis block. * Many unit tests are adapted to use the `InitialCheckpoint` generator in order to comply with the above restriction. ### Comments Merge PR #3046 before this one, because this pull request is based on the branch of the former. Co-authored-by: Heinrich Apfelmus <[email protected]>
|
Build failed: |
|
bors retry |
|
Build succeeded: |
iohk-bors bot
added a commit
that referenced
this pull request
Dec 22, 2021
3056: Introduce `Prologue` and `Discoveries` of an address book r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1043 ### Overview Previous work in epic ADP-1043 introduced delta encodings, DBVars, and an embedding of the wallet state and its delta encodings into a database table. It's time to integrate these tools with the wallet code. To facilitate code review, the integration proceeds in a sequence of refactorings that do not change functionality and pass all unit tests. In this step, we begin refactoring the address discovery state (the `s` in `Wallet s`). First, I would like to update our nomenclature and call this state an *address book*. Second, we introduce two new data families `Prologue s` and `Discoveries s`, with the intention that any address book is isomorphic to a pair of these two types, that is `s ~ (Prologue s, Discoveries s)`. The idea is that the Prologue of the address book contains "static" information such as public keys or the address gap (default 20), while the Discoveries of the address book contains information that was gathered during on-chain address discovery. ### Details * We introduce a type class `AddressBookIso` which provides the isomorphism `s ~ (Prologue s, Discoveries s)`. * For this pull request, we avoid making any changes to the `AddressDiscovery.*` hierarchy. Instead, the isomorphisms are implemented using newly defined data families whose representation is close to the types in the database. * The implementation for the sequential states uses `liftPaymentAddress` to convert the payment part to and from the `Address` type. This is ok when convering to/from the database layer, but does not make sense for in-memory data structures. Therefore, further work in this epic will introduce changes in the `AddressDiscovery.*` hierarchy. ### Comments * The overall goal is that, in a future PR, we can represent the wallet state along the lines of `WalletState s ~ (Prologue s, Checkpoints (BlockHeader, UTxO, Discoveries s) `. * Merge PR #3048 before this one, because this pull request is based on the branch of the former. Co-authored-by: Heinrich Apfelmus <[email protected]>
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.
Issue number
ADP-1043
Overview
Previous work in epic ADP-1043 introduced delta encodings, DBVars, and an embedding of the wallet state and its delta encodings into a database table. It's time to integrate these tools with the wallet code. To facilitate code review, the integration proceeds in a sequence of refactorings that do not change functionality and pass all unit tests.
In this step, we implement a suggestion that came up during review where we remove the
singletonfunction in favor offromGenesis, making sure that theCheckpointsstructure always contains a checkpoint for the genesis block.Details
initializeWalletfunction will now throw an exception if it is incorrectly called with a checkpoint that is not at the genesis block.InitialCheckpointgenerator in order to comply with the above restriction.Comments
Merge PR #3046 before this one, because this pull request is based on the branch of the former.