Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Add TBTC monitoring confirmations #592

Merged
merged 19 commits into from
Nov 12, 2020
Merged

Add TBTC monitoring confirmations #592

merged 19 commits into from
Nov 12, 2020

Conversation

lukasz-zimnoch
Copy link
Member

@lukasz-zimnoch lukasz-zimnoch commented Nov 3, 2020

Refs: #574
Depends on: #591
Depends on: keep-network/keep-common#56

Implemented chain confirmations upon monitoring stop events and after executing monitoring actions. This should prevent problems that may occur due to chain reorganizations.

Function `waitForChainConfirmation` has been
moved to a separate file and made public.
This is because it can be reused in some other
places.
Implemented deposit state confirmations
upon monitoring stop event and action execution
for the retrieve pubkey monitoring.
Implemented deposit state confirmations
upon monitoring stop event and action execution
for remaining monitoring instances.
@pdyraga pdyraga added this to the v1.5.0 milestone Nov 5, 2020
pkg/chain/confirmation.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc_test.go Outdated Show resolved Hide resolved
Base automatically changed from monitoring-dedup to master November 12, 2020 11:28
@lukasz-zimnoch lukasz-zimnoch marked this pull request as ready for review November 12, 2020 11:34
Comment on lines +30 to +36
func newTestTBTC(chain chain.TBTCHandle) *tbtc {
tbtc := newTBTC(chain)

tbtc.blockConfirmations = defaultLocalBlockConfirmations

return tbtc
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go with just:

return &tbtc{
  chain:              chain,
  blockConfirmations: defaultBlockConfirmations,
}

and got rid of newTBTC function that is used only here and in Initalize. See that we are getting nothing with newTBTC and we need to set the field to something else right after it's called.

Copy link
Member Author

@lukasz-zimnoch lukasz-zimnoch Nov 12, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR #600 will give more sense for the newTBTC function. Also, the tbtc struct will probably grow over time.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll leave this discussion open for now but let's don't care about it for this PR and revisit once we merge #600.

pkg/client/client.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
pkg/extensions/tbtc/tbtc.go Outdated Show resolved Hide resolved
@pdyraga pdyraga merged commit 6b0f432 into master Nov 12, 2020
@pdyraga pdyraga deleted the monitoring-confirmations branch November 12, 2020 14:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants