From a65145c6ad454d5e329e896fec9f82f595564ca8 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 19 Nov 2020 15:07:42 +0100 Subject: [PATCH 1/4] Implement signer publication delays Each signer should wait an amount of time before publishing the signature for given keep. The exact amount of time is determined basing on signer index. This should prevent signers to publish the signature in the same time and burn gas for transactions which will be reverted. --- pkg/extensions/tbtc/tbtc.go | 5 +++ pkg/node/node.go | 63 +++++++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/pkg/extensions/tbtc/tbtc.go b/pkg/extensions/tbtc/tbtc.go index dd6623357..1ff21a95e 100644 --- a/pkg/extensions/tbtc/tbtc.go +++ b/pkg/extensions/tbtc/tbtc.go @@ -879,6 +879,11 @@ func (t *tbtc) getSignerActionDelay( return 0, err } + // just in case this function is not invoked in the right context + if signerIndex < 0 { + return 0, fmt.Errorf("signer index is less than zero") + } + return time.Duration(signerIndex) * t.signerActionDelayStep, nil } diff --git a/pkg/node/node.go b/pkg/node/node.go index b94c84d14..c6c557d49 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -27,9 +27,20 @@ import ( var logger = log.Logger("keep-ecdsa") -const monitorKeepPublicKeySubmissionTimeout = 30 * time.Minute -const retryDelay = 1 * time.Second -const blockConfirmations = uint64(12) +const ( + // Determines the delay which should be preserved before retrying + // actions within the signing process. + retryDelay = 1 * time.Second + + // Number of blocks which should elapse before confirming + // the given chain state expectations. + blockConfirmations = uint64(12) + + // Used to calculate the publication delay factor for the given signer index + // to avoid all signers publishing the same signature for given keep at the + // same time. + signerPublicationDelayStep = 5 * time.Minute +) // Node holds interfaces to interact with the blockchain and network messages // transport layer. @@ -327,6 +338,8 @@ func (n *Node) publishSignature( digest [32]byte, signature *ecdsa.Signature, ) error { + n.waitSignerPublicationDelay(keepAddress) + attemptCounter := 0 for { attemptCounter++ @@ -438,6 +451,50 @@ func (n *Node) publishSignature( } } +func (n *Node) waitSignerPublicationDelay( + keepAddress common.Address, +) { + signerIndex, err := n.getSignerIndex(keepAddress) + if err != nil { + logger.Error( + "could not determine signer publication delay: [%v]; "+ + "signer publication delay will not be preserved", + err, + ) + return + } + + // just in case this function is not invoked in the right context + if signerIndex < 0 { + logger.Error( + "could not determine signer publication delay: "+ + "[signer index is less than zero]; "+ + "signer publication delay will not be preserved", + err, + ) + return + } + + delay := time.Duration(signerIndex) * signerPublicationDelayStep + + time.Sleep(delay) +} + +func (n *Node) getSignerIndex(keepAddress common.Address) (int, error) { + members, err := n.ethereumChain.GetMembers(keepAddress) + if err != nil { + return -1, err + } + + for index, member := range members { + if member == n.ethereumChain.Address() { + return index, nil + } + } + + return -1, nil +} + func (n *Node) waitForSignature( keepAddress common.Address, digest [32]byte, From 157b0413fcb651d87caa4f123d319db9f568e967 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Thu, 19 Nov 2020 16:10:07 +0100 Subject: [PATCH 2/4] Improve logging around signer delay waiter --- pkg/node/node.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/node/node.go b/pkg/node/node.go index c6c557d49..d028572ce 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -457,8 +457,9 @@ func (n *Node) waitSignerPublicationDelay( signerIndex, err := n.getSignerIndex(keepAddress) if err != nil { logger.Error( - "could not determine signer publication delay: [%v]; "+ - "signer publication delay will not be preserved", + "could not determine signer publication delay for keep [%s]: "+ + "[%v]; signer publication delay will not be preserved", + keepAddress.String(), err, ) return @@ -467,9 +468,10 @@ func (n *Node) waitSignerPublicationDelay( // just in case this function is not invoked in the right context if signerIndex < 0 { logger.Error( - "could not determine signer publication delay: "+ + "could not determine signer publication delay for keep [%s]: "+ "[signer index is less than zero]; "+ "signer publication delay will not be preserved", + keepAddress.String(), err, ) return @@ -477,6 +479,12 @@ func (n *Node) waitSignerPublicationDelay( delay := time.Duration(signerIndex) * signerPublicationDelayStep + logger.Infof( + "waiting [%v] before publishing signature for keep [%s]", + delay, + keepAddress.String(), + ) + time.Sleep(delay) } From 73cc0ffe0f07952d52f5cc7494158ff8fbad9d6e Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 23 Nov 2020 10:04:57 +0100 Subject: [PATCH 3/4] Improve docs around signature publication delays --- pkg/node/node.go | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/pkg/node/node.go b/pkg/node/node.go index 80a23c2fa..c9fda3497 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -29,7 +29,7 @@ var logger = log.Logger("keep-ecdsa") const ( // Determines the delay which should be preserved before retrying - // actions within the signing process. + // actions within the key generation and signing process. retryDelay = 1 * time.Second // Number of blocks which should elapse before confirming @@ -39,7 +39,7 @@ const ( // Used to calculate the publication delay factor for the given signer index // to avoid all signers publishing the same signature for given keep at the // same time. - signerPublicationDelayStep = 5 * time.Minute + signaturePublicationDelayStep = 5 * time.Minute ) // Node holds interfaces to interact with the blockchain and network messages @@ -320,7 +320,7 @@ func (n *Node) publishSignature( digest [32]byte, signature *ecdsa.Signature, ) error { - n.waitSignerPublicationDelay(keepAddress) + n.waitSignaturePublicationDelay(keepAddress) attemptCounter := 0 for { @@ -433,14 +433,17 @@ func (n *Node) publishSignature( } } -func (n *Node) waitSignerPublicationDelay( +// waitSignaturePublicationDelay waits a certain amount of time appropriately +// for the given signer index to avoid all signers publishing the same signature +// for given keep at the same time. +func (n *Node) waitSignaturePublicationDelay( keepAddress common.Address, ) { signerIndex, err := n.getSignerIndex(keepAddress) if err != nil { - logger.Error( - "could not determine signer publication delay for keep [%s]: "+ - "[%v]; signer publication delay will not be preserved", + logger.Errorf( + "could not determine signature publication delay for keep [%s]: "+ + "[%v]; the signature publication will not be delayed", keepAddress.String(), err, ) @@ -449,17 +452,16 @@ func (n *Node) waitSignerPublicationDelay( // just in case this function is not invoked in the right context if signerIndex < 0 { - logger.Error( - "could not determine signer publication delay for keep [%s]: "+ - "[signer index is less than zero]; "+ - "signer publication delay will not be preserved", + logger.Errorf( + "could not determine signature publication delay for keep [%s], "+ + "signer index is less than zero; the signature publication "+ + "will not be delayed", keepAddress.String(), - err, ) return } - delay := time.Duration(signerIndex) * signerPublicationDelayStep + delay := time.Duration(signerIndex) * signaturePublicationDelayStep logger.Infof( "waiting [%v] before publishing signature for keep [%s]", From 7062199e23d622d448356072570777d3565c28a7 Mon Sep 17 00:00:00 2001 From: Lukasz Zimnoch Date: Mon, 23 Nov 2020 11:58:28 +0100 Subject: [PATCH 4/4] Remove redundant logs about signature submission --- pkg/node/node.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pkg/node/node.go b/pkg/node/node.go index c9fda3497..6421bd545 100644 --- a/pkg/node/node.go +++ b/pkg/node/node.go @@ -372,11 +372,6 @@ func (n *Node) publishSignature( // and there are enough confirmations from the chain. // We are fine, leaving. if !isAwaitingSignature && n.confirmSignature(keepAddress, digest) { - logger.Infof( - "signature for keep [%s] already submitted: [%+x]", - keepAddress.String(), - digest, - ) return nil } @@ -404,11 +399,6 @@ func (n *Node) publishSignature( // confirmations from the chain before making a decision about // leaving the submission process. if !isAwaitingSignature && n.confirmSignature(keepAddress, digest) { - logger.Infof( - "signature for keep [%s] already submitted: [%+x]", - keepAddress.String(), - digest, - ) return nil }