- For now I created `CompactBlockProcessorNG` where I started with
implementation of the state machine. I did it to not break the rest of
the SDK. This change will be merged to the feature branch. And before
it is merged to `main` branch code from `CompactBlockProcessorNG` will
be moved to `CompactBlockProcessor`.
- The new code is not used. It just shows and explains how it is done.
It is proof of concept.
- I did put either commented current code or comment to some places to
explain what should be done there.
- New important data types:
- `ActionContext` is context that can hold any data that needs to be
shared between actions. For example sync ranges or current state.
- `CBPState` is state of the `CompactBlockProcessor`. Each state is
handled by one action. This doesn't apply to terminal states like
`finished` or `failed`.
- `ActionProgress` is very similar to `CompactBlockProgress`.
Different actions reports progress differently and `ActionProgress`
represents this.
- `Action` is protocol that defines API of an action. It has one run
method that executes the code of the action
- CBP first creates actions for (almost) each state in `makeActions()`
method. Then the "magic" is done in `CompactBlockProcessorNG.run()` method.
Here is main loop which takes action for current state and execute it.
It's expected that action does it's work and then updates the context
with new state. And this happens until some terminal state
(`finished`, `failed`, `stopped`) is reached.
- After the transition to state machine API of the
`CompactBlockProcessor` should stay the same. No changes should be
required in `SDKSynchronizer`.
[#700] Add documentation for CompactBlockProcessor state machine
- plantuml tool is used to generate diagram.
[#1054] Add config to state machine CBP
Closes#1054
[#1043] Implement DownloadAction
Closes#1043
[#1049] Implement ValidateAction
Closes#1049
[#1050] Implement ValidateServerAction
Closes#1050
[#1056] Add constructors to state machine CBP
Closes#1056
[#1061] Add failure methods for state machine CBP
Closes#1061
[#1055] Implement retry timer to state machine CBP
Closes#1055
[#1057] Implement start for state machine CBP
Closes#1057
[#1058] Implement stop for state machine CBP
Closes#1058
[#1052] Implement AfterSyncHooksManager when using state machine CBP
Closes#1052
[#1060] Implement wipe for state machine CBP
Closes#1060
[#1059] Implement rewind for state machine CBP
Closes#1059
[#700] Add idle state to CBP state machine
This is required so the CBP can detect start of the sync process.
[#700] Implement sending of some events from CompactBlockProcessorNG
[#700] Implement progress reporting in state machine CBP
[#1045] Implement FetchUTXOsAction
- draft of the fetching
[#1045] Implement FetchUTXOsAction
- updated the way Actions communicate data back to the CBP
- used this mechanism to pass result of utxos fetch so it's passed to the SDKSynchronizer as an Event
[#1042] Implement ComputeSyncRangesAction
Closes#1042
[#700] Implement cache clearing when some actions fail
[#1043] Fix batch range computation in DownloadAction
[#1046] Implement SaplingParamsAction
- action for sapling param files finished
[#1048] Implement ScanDownloadedButUnscannedAction
- scan downloaded but unscanned blocks
[#1047] Implement ScanAction
- scan action with the proper ranges computed
[#1047] Implement ScanAction (#1085)
- fixed logger message
[#1044] Implement EnhanceAction
Closes#1044
[#1041] Implement ClearCacheAction
Closes#1041
[#1040] Implement ClearAlreadyScannedBlocksAction
Closes#1040
[#1039] Implement ChecksBeforeSyncAction
Closes#1039
[#700] Make CBP state machine work
[#1050] Implement ValidateServerAction
- broken tests commented out and tracked in the tickets
- new test for ValidateServerAction
[#1051] Update how progress is computed after switch to state machine
Closes#1051
- new proposal for the progress computation
- OverallProgress value is passed to the Synchronizer as a Float
- OverallProgress is a result of fetch, scan and enhance operations
- Order of actions no longer influences the computation
- Actions report partial updates, CompactBlockProgress actor holds current state and computes the OverallProgress
[#1049] Implement ValidateAction
- synchronizer offline tests updated so it compiles, review is requested in a different ticket
- ValidateAction tests added
- BlockValidator mock generated
[#1047] Implement ScanAction
- ScanAction tests
- refactor of validateAction -> validateServerAction
- generated few more mocks for the DI
[#1045] Implement FetchUTXOsAction
- FetchUTXOsAction tests
- UTXOFetcher mocks
[#1045] Implement FetchUTXOsAction
- enhanced with mocked values and more checks
[#1046] Implement SaplingParamsAction
- SaplingParamsAction tests
- added TODO for TestCoordinator reset()
- SaplingParametersHandler mock added
[#1046] Implement SaplingParamsAction
- SaplingParamsAction tests
- added TODO for TestCoordinator reset()
- SaplingParametersHandler mock added
[#1046] Implement SaplingParamsAction
- rebased so I get functionality of improved mock checks
- enhanced SaplingParamsAction tests
- enhanced ValidateAction tests
- enhanced ScanAction tests
[#1046] Implement SaplingParamsAction
- scanAction tests more checks added
[#1044] Implement EnhanceAction
- EnhanceAction tests focused on 2 different methods:
- decideWhatToDoNext covered separately, decisions where the state machine goes next
- run tests for different cases
- new mocks generated for enhacer
- some typos fixed
[#1044] Implement EnhanceAction (#1107)
- empty assert messages fixed
[#700] Get rid of ScanDownloadedButUnscannedAction
Before the state machine download and scan was called in one loop. And
processing range for one batch was same for both of them. Therefore
there was code which scanned downloaded but not scanned blocks.
But now download and scan are independent. So it is possible to remove
`ScanDownloadedButUnscannedAction`.
[#700] Make NetworkTests compilable
Some tests are disabled for now (list is in #1115). And `NetworkTests`
can be compiled and all the enabled tests work.
[#1043] Implement DownloadAction
- DownloadAction tests
- BlockDownloader mock
[#1043] Implement DownloadAction (#1110)
- support functions set to private
[#1039] Implement ChecksBeforeSyncAction
- ChecksBeforeSyncAction tests
- all support functions in Action tests are set to private
- let _ = -> _ = refactor
- CompactBlockRepository mock added
[#1040] Implement ClearAlreadyScannedBlocksAction
- Implement ClearAlreadyScannedBlocksAction tests
- CompactBlockRepository mock added
[#1041] Implement ClearCacheAction
- ClearCacheAction tests
- CompactBlockRepository mock added
[#1042] Implement ComputeSyncRangesAction
- ComputeSyncRangesAction tests
- fixed all tests after merge of latest SDK changes related InternalSyncProgress
- all actions marked as final class
[#1042] Implement ComputeSyncRangesAction (#1120)
- Custom LatestBlocksDataProviderMock removed from the project
[#1122] Implement FileManager protocol and dependency
- ZcashFileManager implemented
- MigrateLegacyCacheDBAction refactored to be dependent on ZcashFileManager
- ZcashFileManager mock added
[#1122] Implement FileManager protocol and dependency (#1124)
- code cleanup
[#1121] Implement MigrateLegacyCacheDBAction
- MigrateLegacyCacheDBAction tests WIP
- tests naming cleanup
[#1121] Implement MigrateLegacyCacheDBAction
- MigrateLegacyCacheDBAction tests finished
[#700] Fix DarksideTests
Closes#1102
Some tests that can't be compiled are disabled for now. List is in #1126.
This PR contains multiple fixes. Most of the fixes is done in the code.
Not in the tests. That is good news.
Fixes:
- `config` inside `CompactBlockProcessor` can be updated during the
tests. And it must be also updated inside the actions. So
`ConfigProvider` is added and it is wrapper for config that is passed
to any instance of `Action` and provides updated config.
- Fixed `EnhanceAction`. Now it should update all the blocks in the
enhance range even when the remaining count of blocks is lower than
1000.
- Fixed `fail()` and `validationFailed()`. These two were canceling
`syncTask`. But that stopped run loop in a bad way.
[#1129] Final check of all State Machine Action tests
- XTCAsset messages checked
- test naming checked and fixed
[#1126] Fix DarksideTests in state machine branch
Closes#1126
Fix offline tests
Closes#1098Closes#1095Closes#1094
Most of the tests is removed. Either the code that was tested doesn't
exists. Or now tests for state machine actions do this work.
[#1115] Fix NetworkTests in state machine branch
Closes#1115
[#700] Fix progress reporting
Some actions in the sync process may not run. For example there are no
transactions to enhance and therefore there is no enhance progress. And in
cases like this computation of final progress won't work properly.
So let's fake 100% progress at the end of the sync process.
* [#781] This fixes test `testMaxAmountMinusOneSend` by creating two separate
tests:
- testMaxAmountMinusOneSendFails
- testMaxAmountSend
Also includes new functionality that tracks sent transactions so
that users can be notified specifically when they are mined and uses "idea B" of
issue #1033.
closes#1033closes#781
* Fix tests
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.
This change introduces two new protocols: `ClosureSynchronizer` and
`CombineSynchronizer`. These two protocols define API that doesn't use
`async`. So the client can choose exactly which API it wants to use.
This change also introduces two new objects: `ClosureSDKSynchronizer`
and `CombineSDKSynchronizer`. These two implement the respective protocols
mentioned above. Both are structures. Neither of these two keeps any state.
Thanks to this each is very cheap to create. And usage of these two isn't
mutually exclusive. So devs can really choose the best SDK API for each
part of the client app.
[#831] Use async inside of the SDKSynchronizer
- In general lot of methods inside the `SDKSynchronizer` and
`CompactBlockProcessoer` which weren't async are now async. And other
changes are made because of this change.
- `CompactBlockProcessor` no longer uses Combine to communicate with
`SDKSynchronizer`. Reason for this is that Combine doesn't play great
with async. Closure passed to `sink` isn't async.
- Because of this and because of how our tests work (receiving signals
from CBP directly) `CompactBlockProcessor` must be able to handle more
event closures. Not just one. So it now has `eventClosures`
dictionary. It's little bit strange but it works fine.
- `SyncStatus` inside the `SDKSynchronizer` was previously protected by
lock. Now it's protected by simple actor wrapper.
- Changes in tests are minimal. Changes were mady only because
`CompactBlockProcessor` changes from Combine to closures.
[#831] Add tests for ClosureSDKSynchronizer
- Added tests are testing in general if the `ClosuresSDKSynchronizer` is correctly
calling `Synchronizer` and if the values are correctly returned.
- `ClosuresSDKSynchronizer` doesn't contain any logic but it is public
API and we should be sure that it works correctly.
[#831] Add tests for CombineSDKSynchronizer
[#831] Add changelog
Closes#764
- All notification that were previously sent from CompactBlockProcessor are now gone.
- `CompactBlockProcessor` now provides `eventStream` to communicate with `SDKSynchronizer`.
- Added `synchronizerStoredUTXOs` notification.
- Tests are updated to not use notifications anymore. Some tests there
weren't working before are now fixed.
- Added `CompactBlockProcessorEventHandler` utility class. Previously
expectations were subscribing to notifications. This class replaces
this functionality. It subscribes to events from
`CompactBlockProcessor` and fullfill expectations.
Michal Fousek
Francisco Gindre