Skip to content

Commit

Permalink
-Fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
george-dorin committed Jan 20, 2025
1 parent 91dc870 commit b4ce988
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 103 deletions.
2 changes: 1 addition & 1 deletion core/internal/features/ocr2/features_ocr2_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,7 @@ updateInterval = "1m"
contractABI, err2 := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorABI))
require.NoError(t, err2)
apps[0].GetRelayers().LegacyEVMChains().Slice()
ct, err2 := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr)
ct, err2 := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr, apps[0].KeyStore.Eth())
require.NoError(t, err2)
configDigest, epoch, err2 := ct.LatestConfigDigestAndEpoch(testutils.Context(t))
require.NoError(t, err2)
Expand Down
2 changes: 1 addition & 1 deletion core/internal/features/ocr2/features_ocr2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ updateInterval = "1m"
// Assert we can read the latest config digest and epoch after a report has been submitted.
contractABI, err := abi.JSON(strings.NewReader(ocr2aggregator.OCR2AggregatorABI))
require.NoError(t, err)
ct, err := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr)
ct, err := evm.NewOCRContractTransmitter(testutils.Context(t), ocrContractAddress, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].Client(), contractABI, nil, apps[0].GetRelayers().LegacyEVMChains().Slice()[0].LogPoller(), lggr, apps[0].KeyStore.Eth())
require.NoError(t, err)
configDigest, epoch, err := ct.LatestConfigDigestAndEpoch(testutils.Context(t))
require.NoError(t, err)
Expand Down
7 changes: 3 additions & 4 deletions core/services/job/orm.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/logger"
"github.com/smartcontractkit/chainlink/v2/core/null"
"github.com/smartcontractkit/chainlink/v2/core/services/keystore"
"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey"
medianconfig "github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/median/config"
"github.com/smartcontractkit/chainlink/v2/core/services/pipeline"
"github.com/smartcontractkit/chainlink/v2/core/services/relay"
Expand Down Expand Up @@ -347,7 +346,7 @@ func (o *orm) CreateJob(ctx context.Context, jb *Job) error {
}

//Check if secondary transmitter address is used as primary somewhere else
hasLock, err := checkIfKeyHasLock(ctx, tx.keyStore.Eth(), common.HexToAddress(dtTransmitterAddress), ethkey.TXMv1)
hasLock, err := checkIfKeyHasLock(ctx, tx.keyStore.Eth(), common.HexToAddress(dtTransmitterAddress), keystore.TXMv1)
if err != nil {
return err
} else if hasLock {
Expand All @@ -358,7 +357,7 @@ func (o *orm) CreateJob(ctx context.Context, jb *Job) error {

//Check if primary transmitter address is used as secondary somewhere else, don't check for mercury as it uses CSA keys for transmitters
if jb.OCR2OracleSpec.PluginType != types.Mercury {
hasLock, err := checkIfKeyHasLock(ctx, tx.keyStore.Eth(), common.HexToAddress(jb.OCR2OracleSpec.TransmitterID.String), ethkey.TXMv2)
hasLock, err := checkIfKeyHasLock(ctx, tx.keyStore.Eth(), common.HexToAddress(jb.OCR2OracleSpec.TransmitterID.String), keystore.TXMv2)
if err != nil {
return err
} else if hasLock {
Expand Down Expand Up @@ -1769,7 +1768,7 @@ func validateDualTransmissionMeta(meta map[string]interface{}) error {
return nil
}

func checkIfKeyHasLock(ctx context.Context, ks keystore.Eth, address common.Address, usage ethkey.ServiceType) (bool, error) {
func checkIfKeyHasLock(ctx context.Context, ks keystore.Eth, address common.Address, usage keystore.ServiceType) (bool, error) {
rm, err := ks.GetResourceMutex(ctx, address)
if err != nil {
return false, err
Expand Down
10 changes: 5 additions & 5 deletions core/services/keystore/eth.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type Eth interface {
GetStateForKey(ctx context.Context, key ethkey.KeyV2) (ethkey.State, error)
GetStatesForChain(ctx context.Context, chainID *big.Int) ([]ethkey.State, error)
EnabledAddressesForChain(ctx context.Context, chainID *big.Int) (addresses []common.Address, err error)
GetResourceMutex(ctx context.Context, address common.Address) (*ethkey.ResourceMutex, error)
GetResourceMutex(ctx context.Context, address common.Address) (*ResourceMutex, error)

XXXTestingOnlySetState(ctx context.Context, keyState ethkey.State)
XXXTestingOnlyAdd(ctx context.Context, key ethkey.KeyV2)
Expand All @@ -60,24 +60,24 @@ type eth struct {
ds sqlutil.DataSource
subscribers [](chan struct{})
subscribersMu *sync.RWMutex
resourceMutex map[common.Address]*ethkey.ResourceMutex // ResourceMutex is an internal field and ought not be persisted to the database. Its main usage is to verify that the same key is not used for both TXMv1 and TXMv2 (usage in both TXMs will cause nonce drift and will lead to missing transactions). This functionality should be removed after we completely switch to TXMv2
resourceMutex map[common.Address]*ResourceMutex // ResourceMutex is an internal field and ought not be persisted to the database. Its main usage is to verify that the same key is not used for both TXMv1 and TXMv2 (usage in both TXMs will cause nonce drift and will lead to missing transactions). This functionality should be removed after we completely switch to TXMv2
}

// GetResourceMutex gets the resource mutex associates with the address if no resource mutex is found a new one is created
func (ks *eth) GetResourceMutex(ctx context.Context, address common.Address) (*ethkey.ResourceMutex, error) {
func (ks *eth) GetResourceMutex(ctx context.Context, address common.Address) (*ResourceMutex, error) {
ks.lock.RLock()
defer ks.lock.RUnlock()
if ks.isLocked() {
return nil, ErrLocked
}

if ks.resourceMutex == nil {
ks.resourceMutex = make(map[common.Address]*ethkey.ResourceMutex)
ks.resourceMutex = make(map[common.Address]*ResourceMutex)
}

_, exists := ks.resourceMutex[address]
if !exists {
ks.resourceMutex[address] = ethkey.NewResourceMutex()
ks.resourceMutex[address] = NewResourceMutex()
}
return ks.resourceMutex[address], nil
}
Expand Down
67 changes: 0 additions & 67 deletions core/services/keystore/keys/ethkey/models.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package ethkey

import (
"errors"
"sync"
"time"

"github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
Expand All @@ -19,17 +17,6 @@ type State struct {
lastUsed time.Time
}

type ResourceMutex struct {
mu sync.Mutex
activeCount map[ServiceType]int // Tracks active users per service type
}
type ServiceType int

const (
TXMv1 ServiceType = iota
TXMv2
)

func (s State) KeyID() string {
return s.Address.Hex()
}
Expand All @@ -43,57 +30,3 @@ func (s State) LastUsed() time.Time {
func (s *State) WasUsed() {
s.lastUsed = time.Now()
}

// TryLock attempts to lock the resource for the specified service type.
// It returns an error if the resource is locked by a different service type.
func (rm *ResourceMutex) TryLock(serviceType ServiceType) error {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if other service types are using the resource
for otherServiceType, count := range rm.activeCount {
if otherServiceType != serviceType && count > 0 {
return errors.New("resource is locked by another service type")
}
}

// Increment active count for the current service type
rm.activeCount[serviceType]++
return nil
}

// Unlock releases the lock for the service type
func (rm *ResourceMutex) Unlock(serviceType ServiceType) error {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if the service type has an active lock
if rm.activeCount[serviceType] == 0 {
return errors.New("no active lock for this service type")
}

// Decrement active count for the service type
rm.activeCount[serviceType]--
if rm.activeCount[serviceType] == 0 {
delete(rm.activeCount, serviceType)
}
return nil
}

// IsLocked checks if the resource is locked by any service or a specific service type.
func (rm *ResourceMutex) IsLocked(serviceType ServiceType) (bool, error) {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if the resource is locked by the given service type
if count, exists := rm.activeCount[serviceType]; exists && count > 0 {
return true, nil
}
return false, nil
}

func NewResourceMutex() *ResourceMutex {
return &ResourceMutex{
activeCount: make(map[ServiceType]int),
}
}
16 changes: 9 additions & 7 deletions core/services/keystore/mocks/eth.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

66 changes: 66 additions & 0 deletions core/services/keystore/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"math/big"
"sync"
"time"

gethkeystore "github.com/ethereum/go-ethereum/accounts/keystore"
Expand Down Expand Up @@ -423,3 +424,68 @@ func (rawKeys rawKeyRing) keys() (*keyRing, error) {
func adulteratedPassword(password string) string {
return "master-password-" + password
}

type ResourceMutex struct {
mu sync.Mutex
activeCount map[ServiceType]int // Tracks active users per service type
}
type ServiceType int

const (
TXMv1 ServiceType = iota
TXMv2
)

// TryLock attempts to lock the resource for the specified service type.
// It returns an error if the resource is locked by a different service type.
func (rm *ResourceMutex) TryLock(serviceType ServiceType) error {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if other service types are using the resource
for otherServiceType, count := range rm.activeCount {
if otherServiceType != serviceType && count > 0 {
return errors.New("resource is locked by another service type")
}
}

// Increment active count for the current service type
rm.activeCount[serviceType]++
return nil
}

// Unlock releases the lock for the service type
func (rm *ResourceMutex) Unlock(serviceType ServiceType) error {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if the service type has an active lock
if rm.activeCount[serviceType] == 0 {
return errors.New("no active lock for this service type")
}

// Decrement active count for the service type
rm.activeCount[serviceType]--
if rm.activeCount[serviceType] == 0 {
delete(rm.activeCount, serviceType)
}
return nil
}

// IsLocked checks if the resource is locked by any service or a specific service type.
func (rm *ResourceMutex) IsLocked(serviceType ServiceType) (bool, error) {
rm.mu.Lock()
defer rm.mu.Unlock()

// Check if the resource is locked by the given service type
if count, exists := rm.activeCount[serviceType]; exists && count > 0 {
return true, nil
}
return false, nil
}

func NewResourceMutex() *ResourceMutex {
return &ResourceMutex{
activeCount: make(map[ServiceType]int),
}
}
15 changes: 7 additions & 8 deletions core/services/ocrcommon/transmitter.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/forwarders"
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/txmgr"
"github.com/smartcontractkit/chainlink/v2/core/services/keystore"
"github.com/smartcontractkit/chainlink/v2/core/services/keystore/keys/ethkey"
types2 "github.com/smartcontractkit/chainlink/v2/core/services/relay/evm/types"
)

Expand Down Expand Up @@ -94,22 +93,22 @@ func NewOCR2FeedsTransmitter(
strategy types.TxStrategy,
checker txmgr.TransmitCheckerSpec,
chainID *big.Int,
keystore keystore.Eth,
ks keystore.Eth,
dualTransmissionConfig *types2.DualTransmissionConfig,
) (Transmitter, error) {
// Ensure that a keystore is provided.
if keystore == nil {
if ks == nil {
return nil, errors.New("nil keystore provided to transmitter")
}

if hasLock, err := keyHasLock(ctx, keystore, effectiveTransmitterAddress, ethkey.TXMv2); err != nil {
if hasLock, err := keyHasLock(ctx, ks, effectiveTransmitterAddress, keystore.TXMv2); err != nil {
return nil, err
} else if hasLock {
return nil, errors.Errorf("key %s is used as a secondary transmitter in another job. primary and secondary transmitters cannot be mixed", effectiveTransmitterAddress.String())
}

if dualTransmissionConfig != nil {
if hasLock, err := keyHasLock(ctx, keystore, dualTransmissionConfig.TransmitterAddress, ethkey.TXMv1); err != nil {
if hasLock, err := keyHasLock(ctx, ks, dualTransmissionConfig.TransmitterAddress, keystore.TXMv1); err != nil {
return nil, err
} else if hasLock {
return nil, errors.Errorf("key %s is used as a primary transmitter in another job. primary and secondary transmitters cannot be mixed", effectiveTransmitterAddress.String())
Expand All @@ -124,7 +123,7 @@ func NewOCR2FeedsTransmitter(
strategy: strategy,
checker: checker,
chainID: chainID,
keystore: keystore,
keystore: ks,
secondaryContractAddress: dualTransmissionConfig.ContractAddress,
secondaryFromAddress: dualTransmissionConfig.TransmitterAddress,
secondaryMeta: dualTransmissionConfig.Meta,
Expand All @@ -141,7 +140,7 @@ func NewOCR2FeedsTransmitter(
strategy: strategy,
checker: checker,
chainID: chainID,
keystore: keystore,
keystore: ks,
},
}, nil
}
Expand Down Expand Up @@ -251,7 +250,7 @@ func (t *ocr2FeedsTransmitter) CreateSecondaryEthTransaction(ctx context.Context
return errors.New("trying to send a secondary transmission on a non dual transmitter")
}

func keyHasLock(ctx context.Context, ks keystore.Eth, address common.Address, service ethkey.ServiceType) (bool, error) {
func keyHasLock(ctx context.Context, ks keystore.Eth, address common.Address, service keystore.ServiceType) (bool, error) {
rm, err := ks.GetResourceMutex(ctx, address)
if err != nil {
return false, err
Expand Down
Loading

0 comments on commit b4ce988

Please sign in to comment.