Closes#1012
- This is the first change for the dependency injection. This change
adds `DIContainer` which is used to register and resolve dependencies.
- Then this change shows how `DIContainer` can be used to resolve
dependencies. And it also shows how to mock dependencies in tests.
- Constructors of `Initializer` are changed to support mocking of
depedendencies. But it doesn't affect public API in any way.
- Depedencies are registered in `Dependencies.setup()`. Then in the code
`container.resolve(SomeType.self)` is called to get instance of the
dependency.
- `ZcashTestCase` class is added. It inherits from `XCTestCase` and
should be used for base class in tests. `ZcashTestCase` takes care of
creation of `DIContainer` for dependencies mocking. In future we can
maybe move there more things that are used in each test.
- Lot is missing here. Not all the code is using dependency injection.
Tests for `DIContainer` are missing. Only `OfflineTests` work now
(other tests can't be even compiled). It will be added in future
changes.
This PR introduces `ZcashError` enum. This enum represents any error
that can be thrown inside of the SDK and outside of the SDK. Also
`ZcashError` is used in `LightWalletGRPCService` and handled in
`CompactBlockProcessor` as example.
Why enum? First I tried this with some structure which contained code,
message and underlyingError. Problem was when some specific place in the
SDK would like to attach some additional data to error. I didn't want to
add some generic dictionary to store anything with the error.
So I used enum to identify error. Each member can have specified amount
of associated values. So specific error can bring some context data. And
it's type safe.
Each error has also a code. Relationship between errors and codes is
1:1. It may looks bit redundant but it's important. The client app now
can choose how to process errors. Either identify those by the error
itself or by code.
Definition or errors and codes is in `ZcashErrorDefinition`. And then
`ZcashError` and `ZcashErrorCode` are generated by Sourcery. Thanks to
this it is easier to maintain the final code. And it gives us ability to
generate any kind of documentation for the errors and codes. I created
simple example of this in this PR. And it doesn't make the SDK
completely dependent on the Sourcery. Final structures aren't super
complicated and can be written manually if needed.
[#923] Update error handling in DatabaseStorageManager.swift
- cleanup of DatabaseStorageManager, not used anymore
- ZcashError for SimpleConnectionProvider
Revert "[#923] Update error handling in DatabaseStorageManager.swift"
This reverts commit 94e028d31a.
Fix script which generates ZcashError
[#922] Update error handling in DatabaseMigrationManager.swift
Closes#922
[#923] Update error handling in DatabaseStorageManager.swift
- The DatabaseStorageManager is not used so I deleted it
- SimpleConnectionProvider's errors updated
Fix tests
[#955] Update error handling in SaplingParameterDownloader
Closes#955
[#936] Update error handling in NotesDao
Closes#936
[#935] Update error handling in BlockDao
Closes#935
[#931] InternalSyncProgress.computeNextState doesn't need to throw
Closes#931
[#950] Update error handling in rust backend
Closes#949Closes#950
[#941] Update error handling in AccountEntity
Closes#941
[#928] Update error handling in FSCompactBlockRepository
Closes#928
[#925] Update error handling in BlockDownloaderService (#974)
- BlockDownloaderService errors updated
[#924] Update error handling in BlockDownloader (#973)
- BlockDownloaderStream nextBlock updated
[#942] Update error handling in TransactionEntity (#977)
- ZcashTransaction init errors converted to the ZcashError
[#939] Update error handling in TransactionDao (#976)
- TransactionRepositoryError removed and replaced with ZcashError
- all TransactionDAO errors converted to ZcashError
[#943] Update error handling in ZcashCompactBlock
Closes#943
[#945] Update error handling in Memo
Closes#945
[#934] Update error handling in Checkpoint
Closes#944Closes#934
[#938] Update error handling in PendingTransactionDao
Closes#938
[#926] Update error handling in BlockEnhancer
- WIP, switching to transaction repository to unblock this ticket
[#926] Update error handling in BlockEnhancer
- enhancer's errors done
[#926] Update error handling in BlockEnhancer (#994)
- cleanup
[#952] Update error handling in DerivationTool
Closes#952
[#932] Update error handling in BlockValidator
Closes#932
[#940] Update error handling in UnspentTransactionOutputDao
Closes#940
[#946] Update error handling in WalletTypes
Closes#946
[#954] Update error handling in WalletTransactionEncoder
Closes#954
[#953] Update error handling in PersistentTransactionManager
Closes#953
[#956] Update error handling in Initializer
Closes#956
[#947] Update error handling in Zatoshi (#996)
- Zatoshi's errors converted to ZcashError
[#927] Update error handling in UTXOFetcher (#997)
- UTXOFetcher resolved
Use let instead of var where applicable
In the SDK `var` was used on places where `let` would be sufficient. And
default strategy in Swift should use use `let` until mutability is
required. So all these places are changed. No logic is changed.
[#933] Update error handling in CompactBlockProcessor
- CompactBlockProcessor's errors refactored to ZcashError
[#933] Update error handling in CompactBlockProcessor #999
- comments addressed
[#951] Update error handling in SDKSynchronizer
- SDKSynchronizer's errors refactored to ZcashError
[#951] Update error handling in SDKSynchronizer (#1002)
- comments resolved
Make DerivationTool constructor public
DerivationTool.init should be public. This was removed by accident when
adopting async.
[#361] Add changelog
- attached to the processor's timer
- added latest scanned time and latest block height
- sample app showing the new values alongside with the %
- unit tests fixed
- latestBlock() API added
- the transaction repository is no longer used to update the exposed values during the syncing
Closes#888.
- `ZcashRustBackend` is actor now. So majority of methods in this actor
are now async.
- Some methods stayed `static` in `ZcashRustBackend`. It would be hard
to pass instance of the `ZcashRustBackend` to the places where these
methods are used in static manner. And it would change lot of APIs.
But it isn't problem from technical perspective because these methods
would be `nonisolated` otherwise.
- Methods `lastError()` and `getLastError()` in `ZcashRustBackend` are
now private. This makes sure that ther won't be aby race condition
between other methods and these two error methods.
- All the methods for which was `lastError()` used in code now throw
error. So `lastError()` is no longer needed outside of the
`ZcashRustBackend`.
- There are in the public API related to `DerivationTool`.
- `DerivationTool` now requires instance of the `ZcashRustBackend`. And
`ZcashRustBackend` isn't public type. So `DerivationTool` doesn't have
any public constructor now. It can be created only via
`Initializer.makeDerivationTool()` instance method.
- `deriveUnifiedSpendingKey()` and `deriveUnifiedFullViewingKey()` in
`DerivationTool` are now async. It is because these are using
`ZcashRustBackend` inside. `DerivationTool` offers alternative
(closure and combine) APIs. But downside is that there is no sync API
to dervie spending key or viewing key.
- Some methods of the `DerivationTool` are now static. These methods
don't use anything that requires instance of the `DerivationTool`
inside.
[#888] Use Sourcery to generate mocks
- I wrote mock for `Synchronizer` manually. And it's tedious and long
and boring work.
- Now `ZcashRustBackendWelding` is changed a lot so it means
`MockRustBackend` must be changed a lot. So I decided to introduce
`sourcery` to generate mocks from protocols so we don't have to do it
manually ever.
- To generate mocks go to `ZcashLightClientKit/Tests/TestUtils/Sourcery`
directory and run `generateMocks.sh` script.
- Your protocol must be mentioned in `AutoMockable.swift` file.
Generated mocks are in `AutoMockable.generated.swift` file.
[#888] Fix Offline tests
- Offline tests target now runs and tests are green.
- There is log of changes in tests. But logic is not changed.
- Updated `AutoMockable.stencil` so sourcery is able to generate mock as
actor when protocol is marked with: `// sourcery: mockActor`.
- Last few updates in `ZcashRustBackendWelding`. In previous PR `rewindCacheToHeight`
methods was overlooked and it didn't throw error.
- Removed `MockRustBackend` and using generated
`ZCashRustBackendWeldingMock` instead.
- Using generated `SynchronizerMock`.
[#888] Fix NetworkTests
- Changed a bit how rust backend mock is used in the tests. Introduced
`RustBackendMockHelper`. There are some state variables that must be
preserved within one instance of the mock. This helper does exactly
this. It keeps this state variables in the memory and helping mock to
work as expected.
[#888] Fix Darkside tests
Create ZcashKeyDeriving internal protocol
Use New DerivationTool that does not require RustBackend
Remove duplicated methods that had been copied over
[#888] Fix potentially broken tests
I broke the tests because I moved `testTempDirectory` from each
`TestCase` to the `Environment`. By this I caused that each tests uses
exactly same URL. Which is directly against purpose of
`testTempDirectory`.
So now each test calls this one and store it to local variable. So each
test has unique URL.
[#888] Add ability to mock nonisolated methods to AutoMockable.stencil
[#888] Add changelog and fix the documentation in ZcashRustBackendWelding
[#888] Rename derivation rust backend protocol and remove static methods
- Renamed `ZcashKeyDeriving` to `ZcashKeyDerivationBackendWelding`. So
the naming scheme is same as for `ZcashRustBackendWelding`.
- `ZcashKeyDerivationBackend` is now struct instead of enum.
- Methods in `ZcashKeyDerivationBackendWelding` (except one) are no
longer static. Because of this the respective methods in
`DerivationTool` aren't also static anymore.
Closes#469.
- Methods defined in `ZcashRustBackendWelding` protocol which are doing
some IO operations are now async.
- `Initializer` no longer have methods to get balance. Only
`Synchronizer` now have these methods. These methods are newly async.
And `Initilializer` doesn't have alternative APIs.
- There is lot of changes in tests but those are mostly adding `await`.
No logic is changed.
Closes#209.
[#845] Introduce ZcashSynchronizerAlias enum
Closes#845.
[#852] SDKSynchronizer using queues label based on the alias
Closes#852.
[#847] Remove posibility to use DatabaseStorageManager as singleton
Closes#847.
[#850] Remove synchronizerConnectionStateChanged notification
Closes#850.
[#855] Add check if the Alias is already used
Closes#855
- Added `UsedAliasesChecker` utility which is used to register aliases
that are in use.
- `prepare()` and `wipe()` methods now check if the current alias can't
be used and if not then `InitializerError.aliasAlreadyInUse` is
thrown/emitted.
- Some public methods that could cause harm if used with the Alias that
is already in use now throw `SynchronizerError.notPrepared`. Thanks to
this the client app is forced to call `prepare()` first. And
`prepare()` does check for the Alias.
- Added tests for new conditions.
[#849] Make InternalSyncProgress aware of the Alias
Closes#849.
[#853] Only instance with default Alias migrates legacy cache DB
Closes#853.
[#851] Apply the Alias to the URLs
Closes#851.
- `Initializer` now updates paths according to alias before paths are
used anywhere in the SDK.
- Paths update can fail. It would be incovenient for the client apps to
handle errors thrown from `Initiliazer` constructor. So the error is
then handled in `SDKSynchronizer.prepare()` or `SDKSynchronizer.wipe()`.
[#846] Stop using SDKMetrics as singleton (#862)
- metrics are not longer a singleton
- tests fixed
- metrics outside init of the synchronizer
[#848] Make logger aware of the alias
- logger is now an instance passed throughout the sdk instead of a static proxy
[#848] Make logger aware of the alias (#868)
- comments addressed
[#848] Make logger aware of the alias (#868)
- returning protocol back
Fix typos
[#856] Add possibility to test multiple synchronizers in the sample app
Closes#856.
- Added `alias` property to `Synchronizer`.
- Added `SyncBlocksListViewController` which provides UI to use multiple
synchronizers at once.
[#209] Add changelog
- Add changelog for #209.
- Overall improve readability of the rendered changelog. Tickets
references are now prefixed with `###` instead of `- `.
Fix compilation
Closes#826
- `viewingKeys` and `walletBirthday` are removed from `Initializer`
constuctor. These parameters are moved to `SDKSynchronizer.prepare`
function.
- Constructor of the `SDKSynchronizer` no longer throws exception.
- Any value emitted from `lastState` stream before `SDKSynchronizer.prepare`
is called has `latestScannedHeight` set to 0.
- `Initializer.initialize` function isn't public anymore. To initialize
SDK call `SDKSynchronizer.prepare` instead.
- Real birthday is now known after the `SDKSynchronizer.prepare` is
called. But this function isn't async and therefore it's hard to
pass birthday from here to `CompactBlockProcessor` in synchronous
manner. We must be sure that it's safe to call start() after prepare
function finishes.
For this reason `CompactBlockProcessor` has `walletBirthdayProvider`
instead of `walletBirthday`. And this new parameter is closure which
reads birthday from `Initiliazer` on each call. Thanks to this
`CompactBlockProcessor` has always the right birthday.
- Some cleanup is done in `TestCoordinator`.
contains no public API changes.
Adds `func shouldClearBlockCacheAndUpdateInternalState() -> BlockHeight?` to `SyncRanges`
so that the compact block processor can advert internal states that are not consistent and
recover from such state.
Closes#821
Closes#795
- Added structure `SaplingParamsSourceURL` which holds source URLs for
sapling-spend file and sapling-output file. URLs are ultimately passed
to `SaplingParameterDownloader`.
- This is done so it's possible to pass different URLs when running
tests.
- sapling-spend file and sapling-output file are in tests bundle. And
when running tests these files are loaded from bundle and not from
network.
Closes#778
- .swiftlint.yml is now used to lint only the code of the SDK.
- .swiftlint_tests.yml is now used to lint only the code of the tests.
This config disables rules that were previously disabled in the code
in lot of tests.
Closes#771
- Renamed `cacheDirectory` param in `FSCompactBlockRepository`
constructor to `fsBlockDbRoot`.
- Pass same paths to `FSCompactBlockRepository` and `FSMetadataStore`
from tests.
Closes#746
- `LightWalletGRPCService` is no longer public. `LightWalletService` is no longer public.
- `LightWalletGRPCService` shouldn't be used dicrectly. Use `LightWalletServiceFactory` to create instance of the service.
- Moved sending of `blockProcessorConnectivityStateChanged` notification to `Initilizer`.
- All the errors from GRPC are mapped to `LightWalletServiceError`. Handling of `GRPCStatus` in the SDK is no longer required.
- `Service` directory (that contains GRPC code) is moved to `Modules/Service`. We can put more code to `Modules/` if we decide
to modularize more code.
Closes https://github.com/zcash/ZcashLightClientKit/issues/697
Closes https://github.com/zcash/ZcashLightClientKit/issues/720
Closes https://github.com/zcash/ZcashLightClientKit/issues/587
Closes https://github.com/zcash/ZcashLightClientKit/issues/667
Closes https://github.com/zcash/ZcashLightClientKit/issues/443
Closes https://github.com/zcash/ZcashLightClientKit/issues/754
- [#790] Fix ShieldFundsTests
Closes#790
Removes comments on `ShieldFundsTests` since those issues have been fixed
Depends on zcash-light-client-ffi changes that adopt newer versions of
librustzcash crates `zcash_primitives 0.10`, `zcash_client_backend 0.7`,
`zcash_proofs 0.10`, `zcash_client_sqlite 0.5.0`.
Also allows wallets to define a shielding_threshold and will set
foundations to customize minimum confirmations for balances, spends
and shielding operations.
**Test Bootstrapping**
- `ZcashCompactBlockDescriptor`: struct that holds functions to
describe blocks as filenames and compare those filenames
`ZcashCompactBlockDescriptor.live` has the actual implementation
but it can be replaced by mocks if needed on Tests
main implementations are held under `FSCompactBlockRepository.filenameDescription` and `FSCompactBlockRepository.filenameComparison` on a separate extention
`DirectoryListingProviders` provide two default implementations of listing a
directory deterministically. `FileManager` does not define a sorting and needs to be done in-memory by calling `.sorted()` on the resulting collection. If this
is a big toll on performance it can be changed to a POSIX implementation but
this is good for now.
`ZcashCompactBlockDescriptor` adds a `height` helper function to
turn a filename into the height of the block stored.
Implemented `func latestHeight() throws -> BlockHeight ` that
returns the blockheight by querying the cache directory in a sorted
fashion and getting the last value and turning the filename into a
`BlockHeight`
Added `Meta` struct to ZcashCompactBlock.
Tests implemented:
- `filterBlockFiles`
- `testClearTheCache`
- `testLatestHeightEmptyCacheThrows`
- `testLatestHeightEmptyCacheThrowsAsync`
- `testRewindEmptyCacheDoesNothing`
- `testRewindEmptyCacheDoesNothingAsync`
- `testWhenBlockIsStoredItFollowsTheDescribedFormat`
- `testWhenBlockIsStoredItFollowsTheFilenameConvention`
- `testGetLatestHeight`
- `testRewindDeletesTheRightBlocks` test
- `testPerformanceExample` test. This isn't a real performance test because the API doesn't work with async/await yet
adopts `shield_funds` shielding threshold parameter
Implements `initBlockMetadataDb` and fix tests
Renames dbCache parameter to `fsBlockDbRoot`. Builds but tests don't pass.
Removes cacheDb uses from code. Testing utilities still persist.
Added needed information in MIGRATING and CHANGELOG.
Added helper to perform deletion of legacy db and creation a the
new file system backed cache.
Renames parameters and changes code where needed.
Network Constants turned into `enum` with static methods.
DeletelastDownloadedBlock helper from initializer
Removes CompactBlockStorage and CompactBlockEntity.
Implements `latestCachedBlockHeight` on rustbackend.
*Replaces dependencies on ZcashRustWelding with `FSMetadataStore`*
This allows the tests to not depend in a particular implementation
of either the MockRustBackend of or ZcashRustBackend. Also provides
a way to test errors properly and switch implementations of critical
areas like `writeBlocks`.
Closes#740
`CompactBlockDownloading` protocol is renamed to `BlockDownloaderService`. `CompactBlockDownloader` is renamed to `BlockDownloaderServiceImpl`.
What is the motivation for this rename?
Goal of this change is to extract code from `CompactBlockProcessor` which is related to downloading. So naturaly it should be called "downloader".
But `CompactBlockDownloader` was already taken. After analysis of `CompactBlockDownloading` and `CompactBlockDownloader` I found out that
`CompactBlockDownloader` doesn't download blocks exactly. It's more like abstraction over `LightWalletService`. And it does very similar job to
`LightWalletService`. That is why I decided to rename it to "service".
What is the motivation for protoco/impl naming scheme?
First `BlockDownloaderService` protocol doesn't describe capability in my eyes. It describe what something is. Also `BlockDownloaderServicing` sounds
strange. I agree that add suffix "Impl" to class name isn't super nice. But it is acceptable. And it makes sense to use nicer type name
(e.g. `BlockDownloaderService`) for protocol because it's used on much more places in the code.
- Added `BlockDownloader` protocol and `BlockDownloaderImpl` class. The code related to downloading was exctracted to this class.
Closes#657
- Now SDK downloads N downloads and scan those. And repeat until sync is
done. This safes lot of space of disk used to store downloaded blocks.
- Most changes are done in `CompactBlockProcessor.processNewBlocks`.
- `SyncRanges` structure is changed a bit.
- `SyncStatus` is changed. SDK now report that it's syncing with
progress. It doesn't report each phase of the sync process separately.
Also appropriate notifications were updated.
Address review comments
- Small changes
- Add tests to check computation of processing ranges for single loop
- Previously we had one range for each sync which was used for each
phase of sync process. Newly there is separate range for each phase of
the sync process.
- I added `InternalSyncProgress` utility. This utility tracks progress
of some phases. And it is able to compute right ranges which should be
used for syncing.
- Some unused download code from `CompactBlockProcessor` is removed.
Fix tests
Address review comments
- Rebase to master
- Update how range for `processBatchFinished()` is computed.
- Refactor `InternalSyncProgress`
- Add tests for `InternalSyncProgress`
Address review comments
Change how latest downloaded block is tracked
- Cache DB is no longer used to track which block was downloaded as
latest. `InternalSyncProgress` is used to do that.
- Thanks to that #660 is fixed. And cache DB can be completely removed
after sync process.
- Added sleep(1) to some darkside tests when latest block height is set.
Otherwise lightwalletd in darkside mode doesn't have time to setup and
tests are flaky.
Fix TransactionEnhancementTests.testBasicEnhancement test
- sync process enhanced to try to download sapling params if needed
- tests fixed
- failing import Crypto -> import CryptoKit
- download params only if there are sapling funds
state for this synchronizer.
also reports the SychronizerState on synchonizerStarted notification.
Add tests that verify the state being reported properly
Fix `BlockBatchValidationTests`
Closes#614
fixes travis ci
Removed `setStartHeight` function
I’m going back in time and this setHeight is not something we are using anywhere else than in the prepare function. This comes from the times we had `initialize(seed:birthday) and we don’t have that anymore
See diff 246d10edaa (diff-414771774e10bc81260094ffcdc7b310a3be48758b2abd2bfae831c83912c02c)
The `func setStartHeight(_ startHeight: BlockHeight)` function assumes that
there might not be a wallet birthday set up or that it might be changed
in-flight which are things that are no longer happening.
remove test warning
Fix test compiler error
- getting rid of the Operation Queue
- the cleanup is needed
- the update of tests is needed
- tested and it successfully finishes the sync process
[476] CompactBlockProcessor to async/await
- old processNewBlocks() removed
[476] CompactBlockProcessor to async/await
- unused operations removed
[476] CompactBlockProcessor to async/await
- unit tests update
[476] CompactBlockProcessor to async/await
- unit tests refactored
[476] CompactBlockProcessor to async/await
- cleanup of deprecated method
[476] CompactBlockProcessor to async/await
- fail(error) was called even for canceled tasks but that must be excluded
[476] CompactBlockProcessor to async/await
- removal of all ZcashOperations from the code (unit test will follow)
[476] CompactBlockProcessor to async/await
- network tests in building and success order again
[476] CompactBlockProcessor to async/await
- offline tests in building and success order
[476] CompactBlockProcessor to async/await (519)
- cleanup of suspending the task
[476] CompactBlockProcessor to async/await (519)
- most comments resolved
[476] CompactBlockProcessor to async/await (519)
- thread safe state for both sync and async context
[476] CompactBlockProcessor to async/await (519)
- fixed build for a sample project
[476] CompactBlockProcessor to async/await (519)
- func testStartNotifiesSuscriptors() reverted
[476] CompactBlockProcessor to async/await (519)
- TODO added to track why we used NSLock instead of an Actor
- Task priority enhanced
[476] CompactBlockProcessor to async/await (519)
- cleanup in Tasks and priorities
Michal Fousek
Lukas Korba
Francisco Gindre
Francisco Gindre
Lukas Korba
Daniel Haight