-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
refactor mercury #11137
refactor mercury #11137
Changes from all commits
4fb93a7
9865e6b
ec3b511
0b3a28c
bb160e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
ocr2keepers "github.com/smartcontractkit/ocr2keepers/pkg/v3/types" | ||
|
||
"github.com/smartcontractkit/chainlink-relay/pkg/services" | ||
|
||
httypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/headtracker/types" | ||
"github.com/smartcontractkit/chainlink/v2/core/chains/evm/logpoller" | ||
evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types" | ||
|
@@ -56,6 +57,10 @@ type BlockSubscriber struct { | |
lggr logger.Logger | ||
} | ||
|
||
func (bs *BlockSubscriber) LatestBlock() *ocr2keepers.BlockKey { | ||
return bs.latestBlock.Load() | ||
} | ||
|
||
Comment on lines
+60
to
+63
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'm not sure why this change was needed. was this moved from somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This blockSubscriber is used in StreamsLookup, to be more precise, StreamsLookup uses only the latestBlock from blockSubscriber. This blockSubscriber is heavy and from a heavy package. we dont want to carry this to the new streams lib. So, in this refactoring, Would like to hear if there is better way to achieve this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good |
||
var _ ocr2keepers.BlockSubscriber = &BlockSubscriber{} | ||
|
||
func NewBlockSubscriber(hb httypes.HeadBroadcaster, lp logpoller.LogPoller, finalityDepth uint32, lggr logger.Logger) *BlockSubscriber { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,56 +5,55 @@ import ( | |
|
||
ocr2keepers "github.com/smartcontractkit/ocr2keepers/pkg/v3/types" | ||
|
||
"github.com/smartcontractkit/chainlink/v2/core/services/ocr2/plugins/ocr2keeper/evm21/mercury" | ||
|
||
"github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/automation_utils_2_1" | ||
iregistry21 "github.com/smartcontractkit/chainlink/v2/core/gethwrappers/generated/i_keeper_registry_master_wrapper_2_1" | ||
) | ||
|
||
type UpkeepFailureReason uint8 | ||
type PipelineExecutionState uint8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dug more on this. The root cause of circular dependency is the Packer interface. The old Packer interface is very convoluted. For mercury, we just need a subset of the functions, so we defined a smaller Packer for mercury in mercury.go.
The first return of UnpackCheckCallbackResult is a PipelineExecutionState. If we keep using this encoding. PipelineExecutionState, then mercury package will have encoding and encoding package will also need mercury.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the plan to address this? We are causing a regression in typing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In order to extract Mercury as a standalone lib in this PR, we changed the return type of IMHO, this |
||
|
||
const ( | ||
// upkeep failure onchain reasons | ||
UpkeepFailureReasonNone UpkeepFailureReason = 0 | ||
UpkeepFailureReasonUpkeepCancelled UpkeepFailureReason = 1 | ||
UpkeepFailureReasonUpkeepPaused UpkeepFailureReason = 2 | ||
UpkeepFailureReasonTargetCheckReverted UpkeepFailureReason = 3 | ||
UpkeepFailureReasonUpkeepNotNeeded UpkeepFailureReason = 4 | ||
UpkeepFailureReasonPerformDataExceedsLimit UpkeepFailureReason = 5 | ||
UpkeepFailureReasonInsufficientBalance UpkeepFailureReason = 6 | ||
UpkeepFailureReasonMercuryCallbackReverted UpkeepFailureReason = 7 | ||
UpkeepFailureReasonRevertDataExceedsLimit UpkeepFailureReason = 8 | ||
UpkeepFailureReasonRegistryPaused UpkeepFailureReason = 9 | ||
UpkeepFailureReasonNone uint8 = 0 | ||
UpkeepFailureReasonUpkeepCancelled uint8 = 1 | ||
UpkeepFailureReasonUpkeepPaused uint8 = 2 | ||
UpkeepFailureReasonTargetCheckReverted uint8 = 3 | ||
UpkeepFailureReasonUpkeepNotNeeded uint8 = 4 | ||
UpkeepFailureReasonPerformDataExceedsLimit uint8 = 5 | ||
UpkeepFailureReasonInsufficientBalance uint8 = 6 | ||
UpkeepFailureReasonMercuryCallbackReverted uint8 = 7 | ||
UpkeepFailureReasonRevertDataExceedsLimit uint8 = 8 | ||
UpkeepFailureReasonRegistryPaused uint8 = 9 | ||
// leaving a gap here for more onchain failure reasons in the future | ||
// upkeep failure offchain reasons | ||
UpkeepFailureReasonMercuryAccessNotAllowed UpkeepFailureReason = 32 | ||
UpkeepFailureReasonTxHashNoLongerExists UpkeepFailureReason = 33 | ||
UpkeepFailureReasonInvalidRevertDataInput UpkeepFailureReason = 34 | ||
UpkeepFailureReasonSimulationFailed UpkeepFailureReason = 35 | ||
UpkeepFailureReasonTxHashReorged UpkeepFailureReason = 36 | ||
UpkeepFailureReasonMercuryAccessNotAllowed uint8 = 32 | ||
UpkeepFailureReasonTxHashNoLongerExists uint8 = 33 | ||
UpkeepFailureReasonInvalidRevertDataInput uint8 = 34 | ||
UpkeepFailureReasonSimulationFailed uint8 = 35 | ||
UpkeepFailureReasonTxHashReorged uint8 = 36 | ||
|
||
// pipeline execution error | ||
NoPipelineError PipelineExecutionState = 0 | ||
CheckBlockTooOld PipelineExecutionState = 1 | ||
CheckBlockInvalid PipelineExecutionState = 2 | ||
RpcFlakyFailure PipelineExecutionState = 3 | ||
MercuryFlakyFailure PipelineExecutionState = 4 | ||
PackUnpackDecodeFailed PipelineExecutionState = 5 | ||
MercuryUnmarshalError PipelineExecutionState = 6 | ||
InvalidMercuryRequest PipelineExecutionState = 7 | ||
InvalidMercuryResponse PipelineExecutionState = 8 // this will only happen if Mercury server sends bad responses | ||
UpkeepNotAuthorized PipelineExecutionState = 9 | ||
NoPipelineError uint8 = 0 | ||
CheckBlockTooOld uint8 = 1 | ||
CheckBlockInvalid uint8 = 2 | ||
RpcFlakyFailure uint8 = 3 | ||
MercuryFlakyFailure uint8 = 4 | ||
PackUnpackDecodeFailed uint8 = 5 | ||
MercuryUnmarshalError uint8 = 6 | ||
InvalidMercuryRequest uint8 = 7 | ||
InvalidMercuryResponse uint8 = 8 // this will only happen if Mercury server sends bad responses | ||
UpkeepNotAuthorized uint8 = 9 | ||
) | ||
|
||
type UpkeepInfo = iregistry21.KeeperRegistryBase21UpkeepInfo | ||
|
||
type Packer interface { | ||
UnpackCheckResult(payload ocr2keepers.UpkeepPayload, raw string) (ocr2keepers.CheckResult, error) | ||
UnpackCheckCallbackResult(callbackResp []byte) (PipelineExecutionState, bool, []byte, uint8, *big.Int, error) | ||
UnpackPerformResult(raw string) (PipelineExecutionState, bool, error) | ||
UnpackCheckCallbackResult(callbackResp []byte) (uint8, bool, []byte, uint8, *big.Int, error) | ||
UnpackPerformResult(raw string) (uint8, bool, error) | ||
UnpackLogTriggerConfig(raw []byte) (automation_utils_2_1.LogTriggerConfig, error) | ||
PackReport(report automation_utils_2_1.KeeperRegistryBase21Report) ([]byte, error) | ||
UnpackReport(raw []byte) (automation_utils_2_1.KeeperRegistryBase21Report, error) | ||
PackGetUpkeepPrivilegeConfig(upkeepId *big.Int) ([]byte, error) | ||
UnpackGetUpkeepPrivilegeConfig(resp []byte) ([]byte, error) | ||
DecodeStreamsLookupRequest(data []byte) (*StreamsLookupError, error) | ||
DecodeStreamsLookupRequest(data []byte) (*mercury.StreamsLookupError, error) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this TODO still relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still relevant, we are not actually using the new lib yet, this is gonna be the next PR.