tBTC monitoring should filter incoming stop events before performing deposit state change confirmation

[lukasz-zimnoch]

Several keep-ecdsa clients logged provide redemption signature monitoring stop event for deposit [0x...] is not confirmed; monitoring will be continued warnings for deposits of which they are not members, thus they shouldn't perform monitoring of those redemptions.

This happens because the monitoringStopFn subscribes for all stop events occurring in tBTC and perform state change confirmations for all deposits, without checking the deposit address is the one being actually observed:

monitoringStopFn := func(
	handler depositEventHandler,
) subscription.EventSubscription {
	// Stop in case the redemption signature has been provided by someone else.
	signatureSubscription := t.handle.OnDepositGotRedemptionSignature(
		func(depositAddress string) {
			if t.waitDepositStateChangeConfirmation(
				depositAddress,
				initialDepositState,
			) {
				handler(depositAddress)
			} else {
				logger.Warningf(
					"provide redemption signature monitoring stop "+
						"event for deposit [%v] is not confirmed; "+
						"monitoring will be continued",
					depositAddress,
				)
			}
		},
	)

	// Stop in case the redemption proof has been provided by someone else.
	redeemedSubscription := t.handle.OnDepositRedeemed(
		func(depositAddress string) {
			if t.waitDepositStateChangeConfirmation(
				depositAddress,
				initialDepositState,
			) {
				handler(depositAddress)
			} else {
				logger.Warningf(
					"provide redemption signature monitoring stop "+
						"event for deposit [%v] is not confirmed; "+
						"monitoring will be continued",
					depositAddress,
				)
			}
		},
	)

	return subscription.NewEventSubscription(
		func() {
			signatureSubscription.Unsubscribe()
			redeemedSubscription.Unsubscribe()
		},
	)
}

Then, if the confirmation succeeds, the monitorAndAct function checks whether the deposit address from the event is equal to the observed deposit address. If so, the stop signal is triggered:

stopEventSubscription := monitoringStopFn(
	func(stopEventDepositAddress string) {
		if depositAddress == stopEventDepositAddress {
			stopEventChan <- struct{}{}
		}
	},
)
defer stopEventSubscription.Unsubscribe()

This logic is correct but a client running a monitoring process will receive multiple stop events in case several deposits switch to the same state nearly at the same time. It shouldn't cause any issues apart from printing the unnecessary logs as the filtering occur at the handler level so no false stop signals won't be propagated.

As a possible fix, we can try to move the filtering to be executed before deposit state change confirmation. Making it at this stage means we won't perform the confirmation process unnecessarily thus logs won't be displayed. However, this needs a deeper investigation as the logic is complex and some undesired side-effects may happen.