Skip to content

Commit

Permalink
core/capabilities/ccip: fix oracle startup logic (#1348)
Browse files Browse the repository at this point in the history 
Ticket: https://smartcontract-it.atlassian.net/browse/CCIP-3078

We can't start bootstrap oracles and plugin oracles on the same
chainlink node. The main reason for this is that we have a singleton
`SingletonPeerWrapper` object which manages all libocr peers for the
application as a whole. Since this singleton only works with a single
peer ID, it creates a single OCR peer, and all streams are created using
this OCR peer. Since streams must have unique config digests for the
same peer object, running a bootstrap oracle and a plugin oracle for the
same config digest will fail because the stream will not be created.

In order to remedy this we match what will be the case in production,
which is:

* Separate bootstrap node, with a peer ID that is either part of the OCR
committee or not (most likely the latter, in order to avoid exporting /
importing the P2P key from one node's DB to another). This bootstrap
node will have to have a DNS name and be publicly accessible over the
internet in order to be accessed by all nodes in the committee, at least
initially, to facilitate peer discovery.
* Plugin node that is more locked down, i.e no public DNS name required.

To enable this, in this PR we:

* Tweak the `launcher` component to launch bootstrap nodes only or
plugin nodes only, and not both. This does NOT rely on the on-chain
bootstrap P2P IDs that are set in the OCR config. This field will be
removed in subsequent PRs.
* Tweak the `OracleCreator` interface to support the above operation

Follow ups:
* Remove the `bootstrapP2PIds` field from the OCR config in
CCIPConfig.sol
  • Loading branch information
@makramkd
makramkd authored Aug 28, 2024
1 parent caa0304 commit 1659b95
Show file tree
Hide file tree
Showing 16 changed files with 550 additions and 1,129 deletions.
48 changes: 32 additions & 16 deletions core/capabilities/ccip/delegate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/smartcontractkit/chainlink-common/pkg/types"
"github.com/smartcontractkit/chainlink-common/pkg/types/query/primitives"

cctypes "github.com/smartcontractkit/chainlink/v2/core/capabilities/ccip/types"
"github.com/smartcontractkit/chainlink/v2/core/chains/legacyevm"
"github.com/smartcontractkit/chainlink/v2/core/config"
kcr "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/keystone/generated/capabilities_registry"
Expand Down Expand Up @@ -134,30 +135,45 @@ func (d *Delegate) ServicesForSpec(ctx context.Context, spec job.Job) (services
ccipConfigBinding,
)

oracleCreator := oraclecreator.New(
ocrKeys,
transmitterKeys,
d.chains,
d.peerWrapper,
spec.ExternalJobID,
spec.ID,
d.isNewlyCreatedJob,
spec.CCIPSpec.PluginConfig,
ocrDB,
d.lggr,
d.monitoringEndpointGen,
bootstrapperLocators,
hcr,
)
// if bootstrappers are provided we assume that the node is a plugin oracle.
// the reason for this is that bootstrap oracles do not need to be aware
// of other bootstrap oracles. however, plugin oracles, at least initially,
// must be aware of available bootstrappers.
var oracleCreator cctypes.OracleCreator
if len(spec.CCIPSpec.P2PV2Bootstrappers) > 0 {
oracleCreator = oraclecreator.NewPluginOracleCreator(
ocrKeys,
transmitterKeys,
d.chains,
d.peerWrapper,
spec.ExternalJobID,
spec.ID,
d.isNewlyCreatedJob,
spec.CCIPSpec.PluginConfig,
ocrDB,
d.lggr,
d.monitoringEndpointGen,
bootstrapperLocators,
hcr,
)
} else {
oracleCreator = oraclecreator.NewBootstrapOracleCreator(
d.peerWrapper,
bootstrapperLocators,
ocrDB,
d.monitoringEndpointGen,
d.lggr,
)
}

capLauncher := launcher.New(
spec.CCIPSpec.CapabilityVersion,
spec.CCIPSpec.CapabilityLabelledName,
ragep2ptypes.PeerID(p2pID.PeerID()),
d.lggr,
hcr,
oracleCreator,
12*time.Second,
oracleCreator,
)

// register the capability launcher with the registry syncer
Expand Down
54 changes: 4 additions & 50 deletions core/capabilities/ccip/launcher/bluegreen.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,11 @@ type blueGreenDeployment struct {
// blue must always be present.
blue cctypes.CCIPOracle

// bootstrapBlue is the bootstrap node of the blue OCR instance.
// Only a subset of the DON will be running bootstrap instances,
// so this may be nil.
bootstrapBlue cctypes.CCIPOracle

// green is the green OCR instance.
// green may or may not be present.
// green must never be present if blue is not present.
// TODO: should we enforce this invariant somehow?
green cctypes.CCIPOracle

// bootstrapGreen is the bootstrap node of the green OCR instance.
// Only a subset of the DON will be running bootstrap instances,
// so this may be nil, even when green is not nil.
bootstrapGreen cctypes.CCIPOracle
}

// ccipDeployment represents blue-green deployments of both commit and exec
Expand All @@ -44,33 +34,21 @@ type ccipDeployment struct {
func (c *ccipDeployment) Close() error {
var err error

// shutdown blue commit instances.
// shutdown blue commit instance.
err = multierr.Append(err, c.commit.blue.Close())
if c.commit.bootstrapBlue != nil {
err = multierr.Append(err, c.commit.bootstrapBlue.Close())
}

// shutdown green commit instances.
// shutdown green commit instance.
if c.commit.green != nil {
err = multierr.Append(err, c.commit.green.Close())
}
if c.commit.bootstrapGreen != nil {
err = multierr.Append(err, c.commit.bootstrapGreen.Close())
}

// shutdown blue exec instances.
// shutdown blue exec instance.
err = multierr.Append(err, c.exec.blue.Close())
if c.exec.bootstrapBlue != nil {
err = multierr.Append(err, c.exec.bootstrapBlue.Close())
}

// shutdown green exec instances.
// shutdown green exec instance.
if c.exec.green != nil {
err = multierr.Append(err, c.exec.green.Close())
}
if c.exec.bootstrapGreen != nil {
err = multierr.Append(err, c.exec.bootstrapGreen.Close())
}

return err
}
Expand All @@ -80,13 +58,7 @@ func (c *ccipDeployment) StartBlue() error {
var err error

err = multierr.Append(err, c.commit.blue.Start())
if c.commit.bootstrapBlue != nil {
err = multierr.Append(err, c.commit.bootstrapBlue.Start())
}
err = multierr.Append(err, c.exec.blue.Start())
if c.exec.bootstrapBlue != nil {
err = multierr.Append(err, c.exec.bootstrapBlue.Start())
}

return err
}
Expand All @@ -96,13 +68,7 @@ func (c *ccipDeployment) CloseBlue() error {
var err error

err = multierr.Append(err, c.commit.blue.Close())
if c.commit.bootstrapBlue != nil {
err = multierr.Append(err, c.commit.bootstrapBlue.Close())
}
err = multierr.Append(err, c.exec.blue.Close())
if c.exec.bootstrapBlue != nil {
err = multierr.Append(err, c.exec.bootstrapBlue.Close())
}

return err
}
Expand All @@ -127,28 +93,16 @@ func (c *ccipDeployment) HandleBlueGreen(prevDeployment *ccipDeployment) error {
var err error
if prevDeployment.commit.green != nil && c.commit.green == nil {
err = multierr.Append(err, prevDeployment.commit.blue.Close())
if prevDeployment.commit.bootstrapBlue != nil {
err = multierr.Append(err, prevDeployment.commit.bootstrapBlue.Close())
}
} else if prevDeployment.commit.green == nil && c.commit.green != nil {
err = multierr.Append(err, c.commit.green.Start())
if c.commit.bootstrapGreen != nil {
err = multierr.Append(err, c.commit.bootstrapGreen.Start())
}
} else {
return fmt.Errorf("invalid blue-green deployment transition")
}

if prevDeployment.exec.green != nil && c.exec.green == nil {
err = multierr.Append(err, prevDeployment.exec.blue.Close())
if prevDeployment.exec.bootstrapBlue != nil {
err = multierr.Append(err, prevDeployment.exec.bootstrapBlue.Close())
}
} else if prevDeployment.exec.green == nil && c.exec.green != nil {
err = multierr.Append(err, c.exec.green.Start())
if c.exec.bootstrapGreen != nil {
err = multierr.Append(err, c.exec.bootstrapGreen.Start())
}
} else {
return fmt.Errorf("invalid blue-green deployment transition")
}
Expand Down
Loading

0 comments on commit 1659b95

Please sign in to comment.