proof courier: general cleanup and refactor, re-try after restart #734
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
guggero
force-pushed
the
proof-courier-cleanup
branch
3 times, most recently
from
8439119 to
96dcb41
Compare
ffranr
requested changes
Dec 19, 2023
guggero
force-pushed
the
proof-courier-cleanup
branch
from
96dcb41 to
5128588
Compare
jharveyb
approved these changes
Dec 20, 2023
guggero
force-pushed
the
proof-courier-cleanup
branch
from
5128588 to
6247fc9
Compare
Now that we have enabled a proof courier for all integration tests, we don't need to conditionally load the config anymore.
With this commit we disentangle the configurations for the two proof couriers, so we can configure them individually in a proper way.
guggero
force-pushed
the
proof-courier-cleanup
branch
from
6247fc9 to
a7dca4e
Compare
ffranr
reviewed
Jan 3, 2024
|
@guggero just two nits, but otherwise I can approve quickly |
Instead of needing to create a special proof courier address and then creating a courier from that, we want to have a central dispatcher that can hold on to the configuration for each of the different couriers. A courier is then created through the dispatcher directly.
guggero
force-pushed
the
proof-courier-cleanup
branch
from
a7dca4e to
f5937f0
Compare
This commit switches the custodian and porter over to using the new proof courier dispatcher. This then allows us to delete a bunch of now unused code related to proof courier addresses.
We didn't re-try using a proof courier to receive an asset after a restart of the daemon. This commit fixes #597.
This commit moves the notification for subscribers about a transfer being complete from the proof courier only part to the general function we end up in if a transfer is complete.
This commit makes sure we only start the proof courier once the on-chain transaction has confirmed. Otherwise we'll run into backoffs for sure until we get the first confirmation.
By reducing the hashmail proof receiver ACK timeout from 15 seconds to 5 seconds, we can save quite some time during the integration tests.
guggero
force-pushed
the
proof-courier-cleanup
branch
from
f5937f0 to
016016b
Compare
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.
Fixes #597.
This PR removes a bunch of TODOs around the way proof couriers are instantiated and receive their configuration.
After the refactor we then fix a bug where we didn't attempt to fetch a proof using a proof courier again after restarting the daemon.