-
Notifications
You must be signed in to change notification settings - Fork 213
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
Sqlite: add checkpoints and transactions to DBLayer #283
Conversation
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.
I have reviewed 30% of this PR - looks good.
Have to finish it
Just _ -> Right <$> do | ||
-- fixme: deleteCascade is not working with persistent-sqlite. | ||
-- Therefore we need to delete related entities as well. | ||
deleteCheckpoints wid |
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.
it would be great if we could implement cascade delete with TH on haskell side.
My understanding is that sqlite does support delete cascade but persistent-sqlite doesn't have this implemented?
Although, I agree that first iteration should be manual and we can implement this later TM.
1a128d9
to
f8ad236
Compare
@rvl "Cascading deletes are not working with persistent-sqlite, which is annoying." Any idea why? Could we work around that by sending a raw SQL query to the SQLite engine in order to setup the proper foreign keys that would allow us to do that? |
ff6e0ad
to
1ef260f
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.
LGTM! There is weeder error (I do not get why frankly speaking) and cascade delete issue raised by Matthias and Ante that I will second - but it can definitely be addressed in another PR
1ef260f
to
977ea26
Compare
bors r+ |
277: Further enhancements to the Bech32 library. r=KtorZ a=jonathanknowles ## Issue Number Issue #238 ## Overview **This PR:** 1. Implements **error location detection** for the Bech32 encoding, based on the JavaScript [reference implementation](https://github.com/sipa/bech32/blob/master/ecc/javascript/bech32_ecc.js). 2. Provides a set of **property tests** to verify that corrupted Bech32 strings are correctly **rejected** by the `decode` function. Corruption is simulated in various ways: * deleting characters * inserting characters * swapping characters * mutating characters ## Caveats 1. The error location detection is **best effort**: * In cases where it **is** possible to unambiguously identify the locations of erroneous characters, the decoder will return the `StringToDecodeContainsInvalidChars` error with a suitable list of locations. * In cases where it's **not** possible to unambiguously identify the locations of erroneous characters, the decoder will return the `StringToDecodeContainsInvalidChars` with the empty list. 2. Currently, there is no automated test to confirm that the `locateErrors` and `syndrome` functions behave identically to the equivalent functions in the reference JavaScript implementation.⚠️ **It is possible that these implementations differ** (and this is something that we should pay attention to). Nevertheless, in the test suite, we verify that **if** the position of an invalid character **is** detected, then it is **detected at the correct location**. 283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. This PR branch is based on #282. # Overview - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. # Comments - I haven't made any decision about what to do with internal functions such as the `Wallet` constructor. - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com> Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Timed out (retrying...) |
283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. This PR branch is based on #282. # Overview - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. # Comments - I haven't made any decision about what to do with internal functions such as the `Wallet` constructor. - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. This PR branch is based on #282. - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. - I haven't made any decision about what to do with internal functions such as the `Wallet` constructor. - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
Timed out |
977ea26
to
32778c1
Compare
I added a commit which fixes the use of internal constructors/accessors by the Sqlite code. |
8c5d985
to
4896094
Compare
- Replace Wallet with unsafeInitWallet - Replace indexedAddresses with addresses/mkAddressPool - Replace PendingIxs(..) with pendingIxsToList and pendingIxsFromList. - Add Index constructor to module exports
The PendingIx set is always sorted by Index.
Uses an MVar for application-level locking. This may be possible to implement using SQLite BEGIN EXCLUSIVE TRANSACTION but I would prefer to try and refine the DBLayer interface first.
bors r+ |
283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. # Overview - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. # Comments - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io> Co-authored-by: Pawel Jakubas <pawel.jakubas@iohk.io>
Build failed |
bors r+ |
1 similar comment
bors r+ |
283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. # Overview - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. # Comments - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) 289: show feature availability in API specification r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> N/A # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [ ] I have removed the"priority" from the specification and added a "status" indicating the availability of a particular feature in the API. # Comments <!-- Additional comments or screenshots to attach if any --> As requested by Chris. Makes it easier for externals to know what's available or not. A preview: ![image](https://user-images.githubusercontent.com/5680256/58074137-bdc26f00-7ba4-11e9-95b5-33ff723e0f2a.png) ![image](https://user-images.githubusercontent.com/5680256/58074150-c6b34080-7ba4-11e9-83a5-943877e14fd7.png) ![image](https://user-images.githubusercontent.com/5680256/58074161-cfa41200-7ba4-11e9-8d83-0948b129e856.png) ![image](https://user-images.githubusercontent.com/5680256/58074177-dd599780-7ba4-11e9-867e-5047012b1f93.png) ![image](https://user-images.githubusercontent.com/5680256/58074188-e5193c00-7ba4-11e9-845c-cacfd34a9fc9.png) <!-- 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 --> 292: Fix Change Calculation (#286) r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #286 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have extended our test to capture the issue highlighted in the bug - [x] I have looked at the test failing - [x] I have fixed the calculation of the change which was... weirdly way off the specification.. # Comments <!-- Additional comments or screenshots to attach if any --> <!-- 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: Pawel Jakubas <pawel.jakubas@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
bors r+ |
283: Sqlite: add checkpoints and transactions to DBLayer r=KtorZ a=rvl Relates to issue #154. # Overview - Implemented saving and loading of transaction history to SQLite - Implemented saving and loading of wallet checkpoints to SQLite, including the state for sequential scheme address discovery. # Comments - The SqliteSpec testing is a bit light. However, there should be enough implemented for all the DBSpec tests to work. Also I plan to finish the QSM tests tomorrow which should provide even more coverage. - Cascading deletes are not working with persistent-sqlite, which is annoying. - DB indexes are missing on some fields. (Needs some custom SQL, can be fixed later) 289: show feature availability in API specification r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> N/A # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [ ] I have removed the"priority" from the specification and added a "status" indicating the availability of a particular feature in the API. # Comments <!-- Additional comments or screenshots to attach if any --> As requested by Chris. Makes it easier for externals to know what's available or not. A preview: ![image](https://user-images.githubusercontent.com/5680256/58074137-bdc26f00-7ba4-11e9-95b5-33ff723e0f2a.png) ![image](https://user-images.githubusercontent.com/5680256/58074150-c6b34080-7ba4-11e9-83a5-943877e14fd7.png) ![image](https://user-images.githubusercontent.com/5680256/58074161-cfa41200-7ba4-11e9-8d83-0948b129e856.png) ![image](https://user-images.githubusercontent.com/5680256/58074177-dd599780-7ba4-11e9-867e-5047012b1f93.png) ![image](https://user-images.githubusercontent.com/5680256/58074188-e5193c00-7ba4-11e9-845c-cacfd34a9fc9.png) <!-- 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 --> 292: Fix Change Calculation (#286) r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #286 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have extended our test to capture the issue highlighted in the bug - [x] I have looked at the test failing - [x] I have fixed the calculation of the change which was... weirdly way off the specification.. # Comments <!-- Additional comments or screenshots to attach if any --> <!-- 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 --> 296: Improve bech32 coveralls r=KtorZ a=piotr-iohk # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [ ] I have replaced `isLeft` with `Left ...` here and there to improve coveralls score a bit # Comments <!-- Additional comments or screenshots to attach if any --> <!-- 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: Pawel Jakubas <pawel.jakubas@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com> Co-authored-by: Piotr Stachyra <piotr.stachyra@iohk.io>
Relates to issue #154.
Overview
Comments