diff --git a/.ubsignore b/.ubsignore new file mode 100644 index 0000000000..bfa13b1342 --- /dev/null +++ b/.ubsignore @@ -0,0 +1,4 @@ +# Deploy scripts use console.log intentionally for progress output and +# readFileSync/writeFileSync appropriately for synchronous deploy workflows. +# These are not production application code; UBS warnings are false positives. +solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts diff --git a/pkg/bitcoin/transaction_builder.go b/pkg/bitcoin/transaction_builder.go index e446f07517..06e595d65d 100644 --- a/pkg/bitcoin/transaction_builder.go +++ b/pkg/bitcoin/transaction_builder.go @@ -148,7 +148,18 @@ func (tb *TransactionBuilder) getScript( ) } - return transaction.Outputs[utxo.Outpoint.OutputIndex].PublicKeyScript, nil + outputIndex := utxo.Outpoint.OutputIndex + if outputIndex >= uint32(len(transaction.Outputs)) { + return nil, fmt.Errorf( + "output index [%d] out of range for transaction [%s] "+ + "with [%d] outputs", + outputIndex, + hash.Hex(InternalByteOrder), + len(transaction.Outputs), + ) + } + + return transaction.Outputs[outputIndex].PublicKeyScript, nil } // AddOutput adds a new transaction's output. diff --git a/pkg/bitcoin/transaction_builder_test.go b/pkg/bitcoin/transaction_builder_test.go index 246e70cd51..96adf8dede 100644 --- a/pkg/bitcoin/transaction_builder_test.go +++ b/pkg/bitcoin/transaction_builder_test.go @@ -4,6 +4,7 @@ import ( "fmt" "math/big" "reflect" + "strings" "testing" "github.com/keep-network/keep-core/internal/testutils" @@ -110,6 +111,35 @@ func TestTransactionBuilder_AddPublicKeyHashInput(t *testing.T) { } } +func TestTransactionBuilder_AddInputReturnsErrorForOutOfRangeOutputIndex( + t *testing.T, +) { + localChain := newLocalChain() + builder := NewTransactionBuilder(localChain) + + inputTransaction := transactionFrom( + t, + "01000000012d4e0b1ef0bf21eed32f6e2f11353b78534dcf21852d506f6f53b64bb5c6b4c500000000c84730440220590e998a5c28965fd442e700445a60c494124fdbb8aa39cc20c04f2aedadb1a602206acb2f852cd7adea65fe9209024e18d2d6ccac0b1e45c61d80c9bcd62f3e5a12012103989d253b17a6a0f41838b84ff0d20e8898f9d7b1a98f2564da4cc29dcf8581d94c5c14934b98637ca318a4d6e7ca6ffd1690b8e77df6377508f9f0c90d000395237576a9148db50eb52063ea9d98b3eac91489a90f738986f68763ac6776a914e257eccafbc07c381642ce6e7e55120fb077fbed880448f2b262b175ac68ffffffff0110400000000000001976a9148db50eb52063ea9d98b3eac91489a90f738986f688ac00000000", + ) + if err := localChain.addTransaction(inputTransaction); err != nil { + t.Fatal(err) + } + + err := builder.AddPublicKeyHashInput(&UnspentTransactionOutput{ + Outpoint: &TransactionOutpoint{ + TransactionHash: inputTransaction.Hash(), + OutputIndex: 3, + }, + Value: 16400, + }) + if err == nil { + t.Fatal("expected out-of-range output index error") + } + if !strings.Contains(err.Error(), "output index [3] out of range") { + t.Fatalf("unexpected error: [%v]", err) + } +} + func TestTransactionBuilder_AddScriptHashInput(t *testing.T) { var tests = map[string]struct { inputTransactionHex string diff --git a/pkg/firewall/firewall.go b/pkg/firewall/firewall.go index f657870659..e96df54366 100644 --- a/pkg/firewall/firewall.go +++ b/pkg/firewall/firewall.go @@ -52,11 +52,6 @@ func (al *AllowList) Contains(operatorPublicKey *operator.PublicKey) bool { var EmptyAllowList = NewAllowList([]*operator.PublicKey{}) const ( - // PositiveIsRecognizedCachePeriod is the time period the cache maintains - // the positive result of the last `IsRecognized` checks. - // We use the cache to minimize calls to the on-chain client. - PositiveIsRecognizedCachePeriod = 12 * time.Hour - // NegativeIsRecognizedCachePeriod is the time period the cache maintains // the negative result of the last `IsRecognized` checks. // We use the cache to minimize calls to the on-chain client. @@ -74,7 +69,6 @@ func AnyApplicationPolicy( return &anyApplicationPolicy{ applications: applications, allowList: allowList, - positiveResultCache: cache.NewTimeCache(PositiveIsRecognizedCachePeriod), negativeResultCache: cache.NewTimeCache(NegativeIsRecognizedCachePeriod), } } @@ -82,7 +76,6 @@ func AnyApplicationPolicy( type anyApplicationPolicy struct { applications []Application allowList *AllowList - positiveResultCache *cache.TimeCache negativeResultCache *cache.TimeCache } @@ -90,8 +83,10 @@ type anyApplicationPolicy struct { // the network. The operator can join the network if it is an allowlisted node // or it is a non-allowlisted node but it is recognized as eligible by any of // the applications. Nil is returned on a successful validation, error otherwise. -// Due to performance reasons, the results of validations for non-allowlisted -// nodes are stored in a cache for a certain amount of time. +// Due to performance reasons, negative validation results for non-allowlisted +// nodes are stored in a cache for a certain amount of time. Positive results +// are intentionally not cached so that operator revocations take effect on the +// next Validate call rather than after a cache TTL. func (aap *anyApplicationPolicy) Validate( remotePeerPublicKey *operator.PublicKey, ) error { @@ -100,23 +95,17 @@ func (aap *anyApplicationPolicy) Validate( return nil } - // First, check in the in-memory time caches to minimize hits to the ETH client. - // If the Keep client with the given chain address is in the positive result - // cache it means it has been recognized when the last `IsRecognized` was - // executed and caching period has not elapsed yet. Similarly, if the client - // is in the negative result cache it means it hasn't been recognized. + // First, check in the in-memory time cache to minimize hits to the ETH client. + // If the client is in the negative result cache it means it hasn't been + // recognized when the last `IsRecognized` was executed and caching period + // has not elapsed yet. // // If the caching period elapsed, cache checks will return false and we // have to ask the chain about the current status. - aap.positiveResultCache.Sweep() aap.negativeResultCache.Sweep() remotePeerPublicKeyHex := remotePeerPublicKey.String() - if aap.positiveResultCache.Has(remotePeerPublicKeyHex) { - return nil - } - if aap.negativeResultCache.Has(remotePeerPublicKeyHex) { return errNotRecognized } @@ -143,9 +132,5 @@ func (aap *anyApplicationPolicy) Validate( return errNotRecognized } - // Add this address to the positive result cache. - // `IsRecognized` will not be called again for the entire caching period. - aap.positiveResultCache.Add(remotePeerPublicKeyHex) - return nil } diff --git a/pkg/firewall/firewall_test.go b/pkg/firewall/firewall_test.go index 6354aec87b..6ba6071db2 100644 --- a/pkg/firewall/firewall_test.go +++ b/pkg/firewall/firewall_test.go @@ -17,7 +17,6 @@ func TestValidate_PeerNotRecognized_NoApplications(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -45,7 +44,6 @@ func TestValidate_PeerNotRecognized_MultipleApplications(t *testing.T) { newMockApplication(), newMockApplication()}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -72,7 +70,6 @@ func TestValidate_PeerRecognized_FirstApplicationRecognizes(t *testing.T) { application, newMockApplication()}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -101,7 +98,6 @@ func TestValidate_PeerRecognized_SecondApplicationRecognizes(t *testing.T) { newMockApplication(), application}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -140,7 +136,6 @@ func TestValidate_PeerNotRecognized_FirstApplicationReturnedError(t *testing.T) application1, application2}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -148,7 +143,7 @@ func TestValidate_PeerNotRecognized_FirstApplicationReturnedError(t *testing.T) testutils.AssertAnyErrorInChainMatchesTarget(t, applicationError, err) } -func TestValidate_PeerRecognized_Cached(t *testing.T) { +func TestValidate_PeerRecognized_Rechecked(t *testing.T) { _, peerOperatorPublicKey, err := operator.GenerateKeyPair( local_v1.DefaultCurve, ) @@ -165,7 +160,6 @@ func TestValidate_PeerRecognized_Cached(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -175,16 +169,15 @@ func TestValidate_PeerRecognized_Cached(t *testing.T) { } // Ensure the application does not recognize the operator anymore. - // Validation should still succeed, since the cached result should be used. + // Validation should fail because positive results are rechecked to avoid + // accepting peers whose application recognition was revoked. application.setIsRecognized(peerOperatorPublicKey, result{ isRecognized: false, err: nil, }) err = policy.Validate(peerOperatorPublicKey) - if err != nil { - t.Fatal(err) - } + testutils.AssertErrorsSame(t, errNotRecognized, err) } func TestValidate_PeerNotRecognized_CacheEmptied(t *testing.T) { @@ -204,7 +197,6 @@ func TestValidate_PeerNotRecognized_CacheEmptied(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -239,7 +231,6 @@ func TestValidate_PeerNotRecognized_Cached(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -274,7 +265,6 @@ func TestValidate_PeerRecognized_CacheEmptied(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{application}, allowList: EmptyAllowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } @@ -312,7 +302,6 @@ func TestValidate_PeerIsAllowlistedNode(t *testing.T) { policy := &anyApplicationPolicy{ applications: []Application{newMockApplication()}, allowList: allowList, - positiveResultCache: cache.NewTimeCache(cachingPeriod), negativeResultCache: cache.NewTimeCache(cachingPeriod), } diff --git a/pkg/maintainer/spv/spv.go b/pkg/maintainer/spv/spv.go index 990d8b0ec0..f842821c84 100644 --- a/pkg/maintainer/spv/spv.go +++ b/pkg/maintainer/spv/spv.go @@ -272,6 +272,15 @@ func isInputCurrentWalletsMainUTXO( if err != nil { return false, fmt.Errorf("failed to get previous transaction: [%v]", err) } + if fundingOutputIndex >= uint32(len(previousTransaction.Outputs)) { + return false, fmt.Errorf( + "funding output index [%d] out of range for transaction [%s] "+ + "with [%d] outputs", + fundingOutputIndex, + fundingTxHash.String(), + len(previousTransaction.Outputs), + ) + } fundingOutputValue := previousTransaction.Outputs[fundingOutputIndex].Value // Assume the input is the main UTXO and calculate hash. diff --git a/pkg/maintainer/spv/spv_test.go b/pkg/maintainer/spv/spv_test.go index 94c2084d11..6f11fd6e2b 100644 --- a/pkg/maintainer/spv/spv_test.go +++ b/pkg/maintainer/spv/spv_test.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "math/big" "reflect" + "strings" "testing" "github.com/keep-network/keep-core/internal/testutils" @@ -217,6 +218,55 @@ func TestUniqueWalletPublicKeyHashes(t *testing.T) { } } +func TestIsInputCurrentWalletsMainUTXO_OutOfRangeFundingOutput(t *testing.T) { + bytesFromHex := func(str string) []byte { + value, err := hex.DecodeString(str) + if err != nil { + t.Fatal(err) + } + + return value + } + + txFromHex := func(str string) *bitcoin.Transaction { + transaction := new(bitcoin.Transaction) + err := transaction.Deserialize(bytesFromHex(str)) + if err != nil { + t.Fatal(err) + } + + return transaction + } + + btcChain := newLocalBitcoinChain() + fundingTransaction := txFromHex( + "0100000000010110a15e879b7e8b07df62772579a64bf2b409409bbcc8bc2c7f6e39" + + "31dc615e920100000000ffffffff02042900000000000017a9143ec459d0f3c29286" + + "ae5df5fcc421e2786024277e87b4121600000000001600148db50eb52063ea9d98b3" + + "eac91489a90f738986f6024830450221009740ad12d2e74c00ccb4741d533d2ecd69" + + "02289144c4626508afb61eed790c97022006e67179e8e2a63dc4f1ab758867d8bbfe" + + "0a2b67682be6dadfa8e07d3b7ba04d012103989d253b17a6a0f41838b84ff0d20e88" + + "98f9d7b1a98f2564da4cc29dcf8581d900000000", + ) + if err := btcChain.BroadcastTransaction(fundingTransaction); err != nil { + t.Fatal(err) + } + + _, err := isInputCurrentWalletsMainUTXO( + fundingTransaction.Hash(), + 2, + [20]byte{}, + btcChain, + newLocalChain(), + ) + if err == nil { + t.Fatal("expected out-of-range funding output error") + } + if !strings.Contains(err.Error(), "funding output index [2] out of range") { + t.Fatalf("unexpected error: [%v]", err) + } +} + func TestIsInputCurrentWalletsMainUTXO(t *testing.T) { bytesFromHex := func(str string) []byte { value, err := hex.DecodeString(str) diff --git a/pkg/net/retransmission/retransmission.go b/pkg/net/retransmission/retransmission.go index c675d4df4e..ceaed16c4d 100644 --- a/pkg/net/retransmission/retransmission.go +++ b/pkg/net/retransmission/retransmission.go @@ -14,6 +14,11 @@ import ( "github.com/keep-network/keep-core/pkg/net" ) +// maxCachedMessageIDs bounds the retransmission deduplication window. It is +// sized to accommodate the expected number of concurrent protocol participants +// multiplied by the retransmission count per phase, with headroom for bursts. +const maxCachedMessageIDs = 10000 + // RetransmitFn represents a retransmission routine. type RetransmitFn func() error @@ -44,7 +49,9 @@ func ScheduleRetransmissions( // enhances it with functionality allowing to handle retransmissions. // The returned handler filters out retransmissions and calls the delegate // handler only if the received message is not a retransmission or if it is -// a retransmission but it has not been seen by the original handler yet. +// a retransmission but it has not been seen within the last +// maxCachedMessageIDs unique message IDs. Messages older than that window may +// be re-processed if their IDs have been evicted from the cache. // The returned handler is thread-safe. // // Retransmissions are identified by sender transport ID and message sequence @@ -52,8 +59,20 @@ func ScheduleRetransmissions( // considered the same. Handler can not be reused between channels if sequence // number of message is local for channel. func WithRetransmissionSupport(delegate func(m net.Message)) func(m net.Message) { + return withRetransmissionSupport(delegate, maxCachedMessageIDs) +} + +func withRetransmissionSupport( + delegate func(m net.Message), + maxCacheSize int, +) func(m net.Message) { + if maxCacheSize <= 0 { + maxCacheSize = 1 + } + mutex := &sync.Mutex{} - cache := make(map[string]bool) + cache := make(map[string]struct{}) + messageIDs := make([]string, 0, maxCacheSize) return func(message net.Message) { messageID := fmt.Sprintf( @@ -62,12 +81,25 @@ func WithRetransmissionSupport(delegate func(m net.Message)) func(m net.Message) message.Seqno(), ) - mutex.Lock() - _, seen := cache[messageID] - if !seen { - cache[messageID] = true - } - mutex.Unlock() + // The lock is acquired and released inside the closure so that it is + // not held during the delegate call, which may be slow or re-entrant. + seen := func() bool { + mutex.Lock() + defer mutex.Unlock() + + _, seen := cache[messageID] + if !seen { + if len(cache) >= maxCacheSize { + delete(cache, messageIDs[0]) + messageIDs[0] = "" + messageIDs = messageIDs[1:] + } + + cache[messageID] = struct{}{} + messageIDs = append(messageIDs, messageID) + } + return seen + }() if !seen { delegate(message) diff --git a/pkg/net/retransmission/retransmission_test.go b/pkg/net/retransmission/retransmission_test.go index 97c4191a1d..24c0ed3f88 100644 --- a/pkg/net/retransmission/retransmission_test.go +++ b/pkg/net/retransmission/retransmission_test.go @@ -2,6 +2,8 @@ package retransmission import ( "context" + "fmt" + "sync" "sync/atomic" "testing" "time" @@ -78,6 +80,94 @@ func TestHandlerReceiveRetransmissions(t *testing.T) { } } +func TestHandlerEvictsOldRetransmissions(t *testing.T) { + var received []net.Message + + handler := withRetransmissionSupport(func(message net.Message) { + received = append(received, message) + }, 2) + + firstMessage := &mockNetworkMessage{senderID: "a", seqno: 1} + + handler(firstMessage) + handler(&mockNetworkMessage{senderID: "a", seqno: 2}) + handler(&mockNetworkMessage{senderID: "a", seqno: 3}) + handler(firstMessage) + + if len(received) != 4 { + t.Fatalf( + "unexpected number of accepted messages\nactual: [%v]\nexpected: [4]", + len(received), + ) + } +} + +func TestHandlerEvictsAcrossMultipleCycles(t *testing.T) { + // cacheSize=3. Two full eviction cycles: seqno 1-6 all accepted (6 total). + // After seqno 6, cache holds [4,5,6] and seqno 1-3 have been evicted. + // Retransmitting 4, 5, 6 must be filtered (still cached). + // Re-sending 1, 2, 3 must be accepted (evicted from cache). + var received []net.Message + + handler := withRetransmissionSupport(func(message net.Message) { + received = append(received, message) + }, 3) + + for i := uint64(1); i <= 6; i++ { + handler(&mockNetworkMessage{senderID: "a", seqno: i}) + } + + // Still in cache -- must be filtered. + handler(&mockNetworkMessage{senderID: "a", seqno: 4}) + handler(&mockNetworkMessage{senderID: "a", seqno: 5}) + handler(&mockNetworkMessage{senderID: "a", seqno: 6}) + + // Evicted -- must be re-accepted. + handler(&mockNetworkMessage{senderID: "a", seqno: 1}) + handler(&mockNetworkMessage{senderID: "a", seqno: 2}) + handler(&mockNetworkMessage{senderID: "a", seqno: 3}) + + if len(received) != 9 { + t.Fatalf( + "unexpected number of accepted messages\nactual: [%v]\nexpected: [9]", + len(received), + ) + } +} + +func TestHandlerConcurrentAccess(t *testing.T) { + var mu sync.Mutex + var received []net.Message + + handler := withRetransmissionSupport(func(message net.Message) { + mu.Lock() + received = append(received, message) + mu.Unlock() + }, 10) + + const goroutines = 20 + const msgsPerGoroutine = 50 + + var wg sync.WaitGroup + wg.Add(goroutines) + for g := 0; g < goroutines; g++ { + go func(g int) { + defer wg.Done() + for i := 0; i < msgsPerGoroutine; i++ { + handler(&mockNetworkMessage{ + senderID: fmt.Sprintf("peer-%d", g), + seqno: uint64(i), + }) + } + }(g) + } + wg.Wait() + + if len(received) == 0 { + t.Fatal("expected at least one message to be received") + } +} + type mockNetworkMessage struct { senderID string seqno uint64 diff --git a/pkg/tbtc/wallet.go b/pkg/tbtc/wallet.go index 1da076356b..9ad8b7e06f 100644 --- a/pkg/tbtc/wallet.go +++ b/pkg/tbtc/wallet.go @@ -699,6 +699,12 @@ func EnsureWalletSyncedBetweenChains( // transaction's inputs must refer to revealed deposits. We can // check one input. If it points to a revealed deposit, that means // the given transaction is produced by our wallet. + if len(transaction.Inputs) == 0 { + return fmt.Errorf( + "transaction with hash [%s] has no inputs", + utxo.Outpoint.TransactionHash.String(), + ) + } input := transaction.Inputs[0] _, isDeposit, err := bridgeChain.GetDepositRequest( input.Outpoint.TransactionHash, diff --git a/pkg/tbtc/wallet_test.go b/pkg/tbtc/wallet_test.go index 802e3aed3f..600d75d8b5 100644 --- a/pkg/tbtc/wallet_test.go +++ b/pkg/tbtc/wallet_test.go @@ -8,16 +8,17 @@ import ( "encoding/binary" "encoding/hex" "fmt" - "github.com/keep-network/keep-core/pkg/chain" - "github.com/keep-network/keep-core/pkg/protocol/group" "math/big" "reflect" + "strings" "sync" "testing" "time" "github.com/keep-network/keep-core/internal/testutils" "github.com/keep-network/keep-core/pkg/bitcoin" + "github.com/keep-network/keep-core/pkg/chain" + "github.com/keep-network/keep-core/pkg/protocol/group" "github.com/keep-network/keep-core/pkg/tecdsa" ) @@ -170,6 +171,42 @@ func TestWalletDispatcher_Dispatch(t *testing.T) { } } +func TestEnsureWalletSyncedBetweenChains_TransactionWithoutInputs(t *testing.T) { + walletPublicKeyHash := [20]byte{} + outputScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + malformedTransaction := &bitcoin.Transaction{ + Version: 1, + Outputs: []*bitcoin.TransactionOutput{ + { + Value: 1000, + PublicKeyScript: outputScript, + }, + }, + } + + btcChain := newLocalBitcoinChain() + if err := btcChain.BroadcastTransaction(malformedTransaction); err != nil { + t.Fatal(err) + } + + err = EnsureWalletSyncedBetweenChains( + walletPublicKeyHash, + nil, + Connect(), + btcChain, + ) + if err == nil { + t.Fatal("expected transaction-without-inputs error") + } + if !strings.Contains(err.Error(), "has no inputs") { + t.Fatalf("unexpected error: [%v]", err) + } +} + func TestDetermineWalletMainUtxo(t *testing.T) { // In this scenario, we are using e6f9d74726b19b75f16fe1e9feaec048aa4fa1d0 // as the wallet public key hash. This PKH translates to two testnet addresses: diff --git a/pkg/tecdsa/retry/retry.go b/pkg/tecdsa/retry/retry.go index 246e8b1fae..798d3bed30 100644 --- a/pkg/tecdsa/retry/retry.go +++ b/pkg/tecdsa/retry/retry.go @@ -305,7 +305,7 @@ func excludeOperatorTriplets( for k := j + 1; k < len(operators); k++ { leftOperator := operators[i] middleOperator := operators[j] - rightOperator := operators[j] + rightOperator := operators[k] // Only include the operators triples that have few enough seats such // that if they were excluded we still have at least diff --git a/pkg/tecdsa/retry/retry_test.go b/pkg/tecdsa/retry/retry_test.go index 24775c4c7e..af394920a2 100644 --- a/pkg/tecdsa/retry/retry_test.go +++ b/pkg/tecdsa/retry/retry_test.go @@ -2,6 +2,7 @@ package retry import ( "fmt" + "math/rand" "reflect" "strings" "testing" @@ -118,6 +119,48 @@ func TestEvaluateRetryParticipantsForKeyGeneration_NotEnoughOperators(t *testing } } +func TestExcludeOperatorTripletsCountsRightOperatorSeats(t *testing.T) { + leftOperator := chain.Address("operator-left") + middleOperator := chain.Address("operator-middle") + rightOperator := chain.Address("operator-right") + + groupMembers := []chain.Address{ + leftOperator, + middleOperator, + rightOperator, + rightOperator, + rightOperator, + rightOperator, + rightOperator, + rightOperator, + rightOperator, + rightOperator, + } + + _, eligibleTripletsCount, selected := excludeOperatorTriplets( + rand.New(rand.NewSource(1)), + groupMembers, + 0, + map[chain.Address]uint{ + leftOperator: 1, + middleOperator: 1, + rightOperator: 8, + }, + []chain.Address{leftOperator, middleOperator, rightOperator}, + 3, + ) + + if selected { + t.Fatal("expected no eligible triplet to be selected") + } + if eligibleTripletsCount != 0 { + t.Fatalf( + "unexpected eligible triplets count\nexpected: [0]\nactual: [%v]", + eligibleTripletsCount, + ) + } +} + func isSubset( t *testing.T, groupMemberRandomizer groupMemberRandomizer, diff --git a/scripts/initialize.sh b/scripts/initialize.sh index b95f80a128..b63d81f7b8 100755 --- a/scripts/initialize.sh +++ b/scripts/initialize.sh @@ -132,30 +132,30 @@ if [ -z "$authorizer" ]; then authorizer=${stake_owner} fi -stake_amount_opt="" +initialize=( + npx hardhat initialize + --network "$NETWORK" + --owner "$stake_owner" + --provider "$staking_provider" + --operator "$operator" + --beneficiary "$beneficiary" + --authorizer "$authorizer" +) + if [ ! -z "$stake_amount" ]; then - stake_amount_opt="--amount ${stake_amount}" + initialize+=(--amount "$stake_amount") fi -authorization_amount_opt="" if [ ! -z "$authorization_amount" ]; then - authorization_amount_opt="--authorization ${authorization_amount}" + initialize+=(--authorization "$authorization_amount") fi -initialize="npx hardhat initialize - --network $NETWORK \ - --owner ${stake_owner} \ - --provider ${staking_provider} \ - --operator ${operator} \ - --beneficiary ${beneficiary} \ - --authorizer ${authorizer}" - printf "${LOG_START}Initializing beacon...${LOG_END}" -cd $KEEP_BEACON_SOL_PATH -eval ${initialize} ${stake_amount_opt} ${authorization_amount_opt} +cd "$KEEP_BEACON_SOL_PATH" +"${initialize[@]}" printf "${LOG_START}Initializing ecdsa...${LOG_END}" -cd $KEEP_ECDSA_SOL_PATH -eval ${initialize} ${stake_amount_opt} ${authorization_amount_opt} +cd "$KEEP_ECDSA_SOL_PATH" +"${initialize[@]}" printf "${DONE_START}Initialization completed!${DONE_END}" diff --git a/solidity/ecdsa/contracts/WalletRegistryGovernance.sol b/solidity/ecdsa/contracts/WalletRegistryGovernance.sol index f4b28c11da..14bd4176f5 100644 --- a/solidity/ecdsa/contracts/WalletRegistryGovernance.sol +++ b/solidity/ecdsa/contracts/WalletRegistryGovernance.sol @@ -440,8 +440,8 @@ contract WalletRegistryGovernance is Ownable { emit AuthorizationDecreaseDelayUpdated(newAuthorizationDecreaseDelay); ( uint96 minimumAuthorization, - uint64 authorizationDecreaseChangePeriod, - + , + uint64 authorizationDecreaseChangePeriod ) = walletRegistry.authorizationParameters(); // slither-disable-next-line reentrancy-no-eth walletRegistry.updateAuthorizationParameters( diff --git a/solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts b/solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts index d0c8d4bf2e..7a4a5920fa 100644 --- a/solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts +++ b/solidity/ecdsa/deploy/16_initialize_allowlist_weights.ts @@ -341,6 +341,11 @@ const func: DeployFunction = async (hre: HardhatRuntimeEnvironment) => { console.warn( "WARNING: Ownership transfer failed. Manual intervention required." ) + throw new Error( + `Ownership transfer failed; manual intervention required: ${ + error instanceof Error ? error.message : String(error) + }` + ) } } else { console.log() diff --git a/solidity/ecdsa/test/WalletRegistryGovernance.test.ts b/solidity/ecdsa/test/WalletRegistryGovernance.test.ts index d5c410d319..4455f3daf9 100644 --- a/solidity/ecdsa/test/WalletRegistryGovernance.test.ts +++ b/solidity/ecdsa/test/WalletRegistryGovernance.test.ts @@ -854,6 +854,16 @@ describe("WalletRegistryGovernance", async () => { before(async () => { await createSnapshot() + await walletRegistryGovernance + .connect(governance) + .beginAuthorizationDecreaseChangePeriodUpdate(456) + + await helpers.time.increaseTime(constants.governanceDelay) + + await walletRegistryGovernance + .connect(governance) + .finalizeAuthorizationDecreaseChangePeriodUpdate() + await walletRegistryGovernance .connect(governance) .beginAuthorizationDecreaseDelayUpdate(123) @@ -875,6 +885,12 @@ describe("WalletRegistryGovernance", async () => { expect(authorizationDecreaseDelay).to.be.equal(123) }) + it("should preserve the authorization decrease change period", async () => { + const { authorizationDecreaseChangePeriod } = + await walletRegistry.authorizationParameters() + expect(authorizationDecreaseChangePeriod).to.be.equal(456) + }) + it("should emit AuthorizationDecreaseDelayUpdated event", async () => { await expect(tx) .to.emit(