Move delegation certificate declaration in a separate table #980
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
This is because certificates are affected by rollbacks and need to be associate to a given slot, unlike other static metadata of a wallet
KtorZ
commented
Nov 7, 2019
| -> Hash "Tx" | ||
| -> ExceptT ErrRemovePendingTx m () | ||
| -- ^ Remove a pending transaction. | ||
|
|
There was a problem hiding this comment.
Just moved up next to other calls related to txs.
KtorZ
commented
Nov 7, 2019
| toJSON = String . toText | ||
|
|
||
| instance FromJSON PoolId where | ||
| parseJSON = aesonFromText "PoolId" |
There was a problem hiding this comment.
🤷♂️ I still don't understand why persistent requires this for field involved in a PrimaryKey. This is non-sense.
KtorZ
commented
Nov 7, 2019
| @@ -506,7 +507,11 @@ instance Eq XPrv where | |||
|
|
|||
| instance Arbitrary (Hash purpose) where | |||
| arbitrary = do | |||
| Hash . convertToBase Base16 . BS.pack <$> vectorOf 32 arbitrary | |||
| Hash . convertToBase Base16 . BS.pack <$> vectorOf 16 arbitrary | |||
There was a problem hiding this comment.
Generate only 16 initial bytes here since the conversion to Base16 will double the underlying size. It doesn't matter much but it gives nicer QC failures with shorter ids.
paweljakubas
reviewed
Nov 8, 2019
| @@ -359,6 +360,9 @@ newDBLayer logConfig trace mDatabaseFile = do | |||
| deleteCheckpoints wid | |||
| [ CheckpointSlot >. point | |||
| ] | |||
| deleteDelegationCertificates wid | |||
| [ CertSlot >. point | |||
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Nov 8, 2019
975: Fix bench-db on windows r=KtorZ a=rvl Relates to #703. # Overview The database benchmark was failing with: ``` cardano-wallet-core-2019.11.6-bench-db.exe: C:\users\rodney\Temp\benf0f5.db: DeleteFile "\\\\?\\C:\\users\\rodney\\Temp\\benf0f5.db": permission denied (Sharing violation.) ``` And: ``` db.exe: <stdout>: commitBuffer: invalid argument (Invalid argument) ``` It works on windows but not under wine (encoding problem). 980: Move delegation certificate declaration in a separate table r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #913 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have moved tracking of delegation "certificate" into a separate SQLite table - [x] I have adjusted `readWalletMetadata` to now pull the delegation status from the right table - [x] I have added a `putDelegationCertificate` function to insert a new row in the database when new certificates are discovered - [x] I have extended the state-machine tests accordingly, with proper tagging # Comments <!-- Additional comments or screenshots to attach if any --> ``` Cardano.Wallet.DB.Sqlite Sqlite State machine tests Sequential +++ OK, passed 400 tests: 65.8% UnsuccessfulReadTxHistory 57.8% SuccessfulReadCheckpoint 57.2% CreateThenList 55.0% TxUnsortedInputs 54.8% TxUnsortedOutputs 49.0% CreateWalletTwice 32.5% ReadTxHistoryAfterDelete 28.5% PutCheckpointTwice 27.8% UnsuccessfulReadCheckpoint 24.8% ReadMetaAfterPutCert 22.5% CreateThreeWallets 18.5% RemovePendingTxTwice 17.5% RolledBackOnce 14.2% SuccessfulReadPrivateKey 13.2% SuccessfulReadTxHistory 12.0% RemoveWalletTwice Parallel # PENDING: No reason given Sqlite State machine (RndState) Sequential state machine tests +++ OK, passed 400 tests: 65.8% UnsuccessfulReadTxHistory 57.8% SuccessfulReadCheckpoint 57.2% CreateThenList 55.0% TxUnsortedInputs 54.8% TxUnsortedOutputs 49.0% CreateWalletTwice 32.5% ReadTxHistoryAfterDelete 28.5% PutCheckpointTwice 27.8% UnsuccessfulReadCheckpoint 24.8% ReadMetaAfterPutCert 22.5% CreateThreeWallets 18.5% RemovePendingTxTwice 17.5% RolledBackOnce 14.2% SuccessfulReadPrivateKey 13.2% SuccessfulReadTxHistory 12.0% RemoveWalletTwice ``` <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Build succeeded |
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
#913
Overview
readWalletMetadatato now pull the delegation status from the right tableputDelegationCertificatefunction to insert a new row in the database when new certificates are discoveredComments