Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FUN-877] make persisting allowlist compatible with older contract #11907

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 47 additions & 11 deletions core/services/gateway/handlers/functions/allowlist/allowlist.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"encoding/hex"
"fmt"
"math/big"
"regexp"
"sync"
"sync/atomic"
"time"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"github.com/pkg/errors"
"golang.org/x/mod/semver"

"github.com/smartcontractkit/chainlink-common/pkg/services"
evmclient "github.com/smartcontractkit/chainlink/v2/core/chains/evm/client"
Expand All @@ -23,9 +25,10 @@ import (
)

const (
defaultStoredAllowlistBatchSize = 1000
defaultOnchainAllowlistBatchSize = 100
defaultFetchingDelayInRangeSec = 1
defaultStoredAllowlistBatchSize = 1000
defaultOnchainAllowlistBatchSize = 100
defaultFetchingDelayInRangeSec = 1
tosContractMinBatchProcessingVersion = "v1.1.0"
)

type OnchainAllowlistConfig struct {
Expand All @@ -38,8 +41,6 @@ type OnchainAllowlistConfig struct {
UpdateTimeoutSec uint `json:"updateTimeoutSec"`
StoredAllowlistBatchSize uint `json:"storedAllowlistBatchSize"`
OnchainAllowlistBatchSize uint `json:"onchainAllowlistBatchSize"`
// StoreAllowedSendersEnabled is a feature flag that enables storing in db a copy of the allowlist.
StoreAllowedSendersEnabled bool `json:"storeAllowedSendersEnabled"`
// FetchingDelayInRangeSec prevents RPC client being rate limited when fetching the allowlist in ranges.
FetchingDelayInRangeSec uint `json:"fetchingDelayInRangeSec"`
}
Expand Down Expand Up @@ -210,7 +211,31 @@ func (a *onchainAllowlist) updateFromContractV1(ctx context.Context, blockNum *b
}

var allowedSenderList []common.Address
if !a.config.StoreAllowedSendersEnabled {
typeAndVersion, err := tosContract.TypeAndVersion(&bind.CallOpts{
Pending: false,
BlockNumber: blockNum,
Context: ctx,
})
if err != nil {
return errors.Wrap(err, "failed to fetch the tos contract type and version")
}

currentVersion, err := ExtractContractVersion(typeAndVersion)
if err != nil {
return fmt.Errorf("failed to extract version: %w", err)
}

if semver.Compare(tosContractMinBatchProcessingVersion, currentVersion) <= 0 {
err = a.syncBlockedSenders(ctx, tosContract, blockNum)
if err != nil {
return errors.Wrap(err, "failed to sync the stored allowed and blocked senders")
}

allowedSenderList, err = a.getAllowedSendersBatched(ctx, tosContract, blockNum)
if err != nil {
return errors.Wrap(err, "failed to get allowed senders in rage")
}
} else {
allowedSenderList, err = tosContract.GetAllAllowedSenders(&bind.CallOpts{
Pending: false,
BlockNumber: blockNum,
Expand All @@ -219,15 +244,15 @@ func (a *onchainAllowlist) updateFromContractV1(ctx context.Context, blockNum *b
if err != nil {
return errors.Wrap(err, "error calling GetAllAllowedSenders")
}
} else {
err = a.syncBlockedSenders(ctx, tosContract, blockNum)

err = a.orm.PurgeAllowedSenders()
Copy link
Contributor

@justinkaseman justinkaseman Feb 2, 2024

Choose a reason for hiding this comment

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

This step to purge the DB will happen on every sync of the stored allow list right?

What is the performance impact of deleting all and re-adding? Because on the new version of the ToS contract this full purge is not needed.

Can we put the full purge behind vs. only deleting blocked users behind a feature flag? How about using the contract's typeAndVersion instead of a job spec change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will only happen for the old ToS contracts that will GetAllAllowedSenders.

I like your idea of checking the contract typeAndVersion instead of a job spec change. Also the naming of the feature flag does not make sense now because its storing anyways. thanks will change that

if err != nil {
return errors.Wrap(err, "failed to sync the stored allowed and blocked senders")
a.lggr.Errorf("failed to purge allowedSenderList: %w", err)
}

allowedSenderList, err = a.getAllowedSendersBatched(ctx, tosContract, blockNum)
err = a.orm.CreateAllowedSenders(allowedSenderList)
if err != nil {
return errors.Wrap(err, "failed to get allowed senders in rage")
a.lggr.Errorf("failed to update stored allowedSenderList: %w", err)
}
}

Expand Down Expand Up @@ -344,3 +369,14 @@ func (a *onchainAllowlist) loadStoredAllowedSenderList() {

a.update(allowedList)
}

func ExtractContractVersion(str string) (string, error) {
pattern := `v(\d+).(\d+).(\d+)`
re := regexp.MustCompile(pattern)

match := re.FindStringSubmatch(str)
if len(match) != 4 {
return "", fmt.Errorf("version not found in string: %s", str)
}
return fmt.Sprintf("v%s.%s.%s", match[1], match[2], match[3]), nil
}
Loading
Loading