Add WalletState, a pure model for the entire wallet
#3099
Conversation
HeinrichApfelmus
changed the title
WalletState, a pure model for the entire wallet
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/WalletState
branch
2 times, most recently
from
222cbe0
to
c649f57
Compare
| let (prologue, wcp) = fromWallet cp | ||
| slot = getSlot wcp | ||
| delta = Just $ Adjust wid | ||
| [ UpdateCheckpoints $ PutCheckpoint slot wcp |
There was a problem hiding this comment.
so basically this is a moment we do persistence, ie. store what is in cp and write to the DB, right? And thanks to construction of Adjust we will only write "difference" data, right? not all data from scratch?
There was a problem hiding this comment.
Essentially yes. More specifically, at this point in the code, we specify the delta that should be applied to the Store.
The actual update is performed by the updateS function of the Store here:
Currently, this function still writes the entire UTxO and discovered addresses for the given checkpoint. In order to change that, we need to a) change the constructor PutCheckpoint to something like PutDifferenceToPreviousCheckpoint and b) change the database schema to accommodate this. One of the next few pull requests will do a), and one of the last commits in this series will be about b).
That said, it is true that thanks to Adjust wid, only the checkpoint for one wallet will be written — not all of them.
There was a problem hiding this comment.
ok, thanks for clarification
| :: forall s. PersistAddressBook s | ||
| => W.WalletId | ||
| -> Store (SqlPersistT IO) (DeltaWalletState s) | ||
| mkStoreWallet wid = |
There was a problem hiding this comment.
I wonder if we can introduce reasonable invariant to make sure DeltaWalletState s is legal/not corrupt (especially in updateS). It could be not the case now (as we have prologue and checkpoints only), but when new things like tx history, pending txs, etc. will be added maybe it is going to be crucial this is the case. Do you have opinion on that?
There was a problem hiding this comment.
That's a good question: As deltas are now first-class values, how can we ensure that deltas da are always paired with the Base da value from which they were derived? 🤔
Unfortunately, I don't have a good answer to that yet, but a couple of thoughts:
-
The status quo has the same problem, so at least we are not doing worse. For example, random calls to
putTxHistorycould also mess up the database, it's the responsibility of the caller ofputTxHistoryto make sure that the call is appropriate for the current state of the database. -
Maybe first-class deltas that can be applied to different values is something we actually want?
- For example, the
DeltaSet1type fromData.Deltasatisfies interesting algebraic properties such asapply [Insert a, Delete a] = apply (Insert a). - In this light, it is sensible to design the delta type so that applying it will never fail. For example,
Data.Set.deletenever fails to return a result, even if the element that is to be deleted is not in the set. An alternative design would have beenData.Set.delete :: Set a -> Maybe (Set a), but that would have been more cumbersome to use. - Given a partial function
changeP :: b -> Maybe b, we can always turn it into a total functionchangeT :: b -> bby definingchangeT b = fromMaybe b $ changeP b. So, the idea would be to always use the second definition when designing a delta type and implementingapply.
- For example, the
-
In the other direction, perhaps it is possible to use the type system to ensure that each delta is paired with the value? In a dependently typed language, one could make the value part of the delta type. That would be very cumbersome, though. But perhaps it is possible to simulate this with some type "tags" that keep track of which delta belongs to which value?
newtype Delta da x y newtype Base da x apply :: Delta da x y -> Base da x -> Base da yHere, the tag
xensures that the base value and the delta belong together. Chances are that managing these tags will involve existential quantification and be somewhat cumbersome as well, but maybe there is some low-hanging fruit here.
For now, I'm leaning towards 1 and 2 — making sure that deltas can always be applied and use property tests to make that sequences of deltas are correctly generated.
There was a problem hiding this comment.
indeed, 1 and 2 sounds reasonable
There was a problem hiding this comment.
lgtm! Another step progressing towards new DB layer. It would be good if @rvl also give it another look before merging. This PR seems to have a tiny risk of destroying anything, we still write whole utxos and discovered addresses as before, schema is intact. I am very fond of how new storage abstractions eat out bite by bite the legacy solution:-)
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/WalletState
branch
from
0ee8753
to
2691be8
Compare
|
bors try |
|
@HeinrichApfelmus I've run e2e suite also on top of this branch and all passed. So lgtm. |
tryTimed out. |
|
bors merge |
3099: Add `WalletState`, a pure model for the entire wallet r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1375 ### 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 introduce a data type `WalletState` which represents the entire wallet state — not just the most recent checkpoint, but _all_ checkpoints. ``` data WalletState s = WalletState { prologue :: Prologue s , checkpoints :: Checkpoints (BlockHeader, UTxO, Discoveries s) } ``` The states for the different wallets currently stored in the `walletsVar` DBVar. Eventually, the data type will become the purely functional in-memory representation of all the data associated with a wallet (though perhaps with a different name). The `DBLayer` type will eventually be replaced by a `Store` for values of this type. The introduction of this type has become possible thanks to the previous separation of the address discovery state `s` into `Prologue s` and `Discoveries s` (PRs #3056, #3068, #3073). ### Details * As the queries in the `DBLayer` will more and more become queries on the in-memory cache `walletsVar` instead of queries on the database table, I have begun to add unit tests for the database tables. Here, I have added a property test for `loadPrologue` and `insertPrologue`. Eventually, these separate tests will become a single generic test for a `Store` of `Table`. ### Comments * One of the next steps will be to replace `UTxO` in the above type by its delta encoding `DeltaUTxO`. This will reduce the memory footprint of the in-memory representation. * The `Cardano.Wallet.DB.Model` module actually implements a pure model for the entire wallet state, including TxHistory etc. When extending the type, we can scavenge from there. Co-authored-by: Heinrich Apfelmus <heinrich.apfelmus@iohk.io>
|
Build failed: Over 100 errors. Seems the connection or cluster completely broke at some point. |
|
bors retry |
3099: Add `WalletState`, a pure model for the entire wallet r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1375 ### 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 introduce a data type `WalletState` which represents the entire wallet state — not just the most recent checkpoint, but _all_ checkpoints. ``` data WalletState s = WalletState { prologue :: Prologue s , checkpoints :: Checkpoints (BlockHeader, UTxO, Discoveries s) } ``` The states for the different wallets currently stored in the `walletsVar` DBVar. Eventually, the data type will become the purely functional in-memory representation of all the data associated with a wallet (though perhaps with a different name). The `DBLayer` type will eventually be replaced by a `Store` for values of this type. The introduction of this type has become possible thanks to the previous separation of the address discovery state `s` into `Prologue s` and `Discoveries s` (PRs #3056, #3068, #3073). ### Details * As the queries in the `DBLayer` will more and more become queries on the in-memory cache `walletsVar` instead of queries on the database table, I have begun to add unit tests for the database tables. Here, I have added a property test for `loadPrologue` and `insertPrologue`. Eventually, these separate tests will become a single generic test for a `Store` of `Table`. ### Comments * One of the next steps will be to replace `UTxO` in the above type by its delta encoding `DeltaUTxO`. This will reduce the memory footprint of the in-memory representation. * The `Cardano.Wallet.DB.Model` module actually implements a pure model for the entire wallet state, including TxHistory etc. When extending the type, we can scavenge from there. Co-authored-by: Heinrich Apfelmus <heinrich.apfelmus@iohk.io>
|
Build failed: |
* Separates the `Prologue` from the `Discoveries` and only stores the latter in the `Checkpoints`. * Intended to include pending transactions, TxHistory, … in the future
HeinrichApfelmus
force-pushed
the
HeinrichApfelmus/ADP-1043/WalletState
branch
from
2691be8
to
cededf8
Compare
|
bors r+ |
3099: Add `WalletState`, a pure model for the entire wallet r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1375 ### 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 introduce a data type `WalletState` which represents the entire wallet state — not just the most recent checkpoint, but _all_ checkpoints. ``` data WalletState s = WalletState { prologue :: Prologue s , checkpoints :: Checkpoints (BlockHeader, UTxO, Discoveries s) } ``` The states for the different wallets currently stored in the `walletsVar` DBVar. Eventually, the data type will become the purely functional in-memory representation of all the data associated with a wallet (though perhaps with a different name). The `DBLayer` type will eventually be replaced by a `Store` for values of this type. The introduction of this type has become possible thanks to the previous separation of the address discovery state `s` into `Prologue s` and `Discoveries s` (PRs #3056, #3068, #3073). ### Details * As the queries in the `DBLayer` will more and more become queries on the in-memory cache `walletsVar` instead of queries on the database table, I have begun to add unit tests for the database tables. Here, I have added a property test for `loadPrologue` and `insertPrologue`. Eventually, these separate tests will become a single generic test for a `Store` of `Table`. ### Comments * One of the next steps will be to replace `UTxO` in the above type by its delta encoding `DeltaUTxO`. This will reduce the memory footprint of the in-memory representation. * The `Cardano.Wallet.DB.Model` module actually implements a pure model for the entire wallet state, including TxHistory etc. When extending the type, we can scavenge from there. Co-authored-by: Heinrich Apfelmus <heinrich.apfelmus@iohk.io> Co-authored-by: IOHK <devops+stack-project@iohk.io>
|
Build failed: |
|
bors retry |
3099: Add `WalletState`, a pure model for the entire wallet r=HeinrichApfelmus a=HeinrichApfelmus ### Issue number ADP-1375 ### 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 introduce a data type `WalletState` which represents the entire wallet state — not just the most recent checkpoint, but _all_ checkpoints. ``` data WalletState s = WalletState { prologue :: Prologue s , checkpoints :: Checkpoints (BlockHeader, UTxO, Discoveries s) } ``` The states for the different wallets currently stored in the `walletsVar` DBVar. Eventually, the data type will become the purely functional in-memory representation of all the data associated with a wallet (though perhaps with a different name). The `DBLayer` type will eventually be replaced by a `Store` for values of this type. The introduction of this type has become possible thanks to the previous separation of the address discovery state `s` into `Prologue s` and `Discoveries s` (PRs #3056, #3068, #3073). ### Details * As the queries in the `DBLayer` will more and more become queries on the in-memory cache `walletsVar` instead of queries on the database table, I have begun to add unit tests for the database tables. Here, I have added a property test for `loadPrologue` and `insertPrologue`. Eventually, these separate tests will become a single generic test for a `Store` of `Table`. ### Comments * One of the next steps will be to replace `UTxO` in the above type by its delta encoding `DeltaUTxO`. This will reduce the memory footprint of the in-memory representation. * The `Cardano.Wallet.DB.Model` module actually implements a pure model for the entire wallet state, including TxHistory etc. When extending the type, we can scavenge from there. Co-authored-by: Heinrich Apfelmus <heinrich.apfelmus@iohk.io> Co-authored-by: IOHK <devops+stack-project@iohk.io>
|
Build failed: |
|
bors retry |
|
Build succeeded: |
Issue number
ADP-1375
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 introduce a data type
WalletStatewhich represents the entire wallet state — not just the most recent checkpoint, but all checkpoints.The states for the different wallets currently stored in the
walletsVarDBVar. Eventually, the data type will become the purely functional in-memory representation of all the data associated with a wallet (though perhaps with a different name). TheDBLayertype will eventually be replaced by aStorefor values of this type.The introduction of this type has become possible thanks to the previous separation of the address discovery state
sintoPrologue sandDiscoveries s(PRs #3056, #3068, #3073).Details
DBLayerwill more and more become queries on the in-memory cachewalletsVarinstead of queries on the database table, I have begun to add unit tests for the database tables. Here, I have added a property test forloadPrologueandinsertPrologue. Eventually, these separate tests will become a single generic test for aStoreofTable.Comments
UTxOin the above type by its delta encodingDeltaUTxO. This will reduce the memory footprint of the in-memory representation.Cardano.Wallet.DB.Modelmodule actually implements a pure model for the entire wallet state, including TxHistory etc. When extending the type, we can scavenge from there.