Initializing S4 plugin #9725

pinebit · 2023-06-29T12:15:55Z

No description provided.

github-actions · 2023-06-29T12:16:23Z

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

bolekk · 2023-06-29T12:51:56Z

core/services/ocr2/delegate.go

+		const (
+			_ int32 = iota
+			thresholdPluginId
+			s4PluingId


bolekk · 2023-06-29T12:56:44Z

core/services/ocr2/delegate.go

+		BinaryNetworkEndpointFactory: d.peerWrapper.Peer2,
+		V2Bootstrappers:              bootstrapPeers,
+		ContractTransmitter:          functionsProvider.ContractTransmitter(),
+		ContractConfigTracker:        functionsProvider.ContractConfigTracker(),


This is not going to work. We need a separate instance of functionsProvider for each plugin with corresponding FunctionsPluginType set.
I wonder if we could do some minor refactor to avoid too much copy-paste for 3 different plugins...

bolekk · 2023-06-29T12:57:52Z

core/services/ocr2/plugins/functions/plugin.go

 		connectorLogger := conf.Logger.Named("GatewayConnector").With("jobName", conf.Job.PipelineSpec.JobName)
+		s4Storage := s4.NewStorage(conf.Logger, *pluginConfig.S4Constraints, s4ORM, utils.NewRealClock())


Accidental change? connectorLogger is used by the line below

bolekk · 2023-06-29T12:59:34Z

core/services/ocr2/plugins/s4/factory.go

@@ -33,7 +33,7 @@ func (f S4ReportingPluginFactory) NewReportingPlugin(rpConfig types.ReportingPlu
 		UniqueReports: false,
 		Limits:        *limits,
 	}
-	plugin, err := NewReportingPlugin(f.Logger, config, f.ORM)
+	plugin, err := NewReportingPluginInstance(f.Logger, config, f.ORM)


I'd avoid changing that for consistency with other factories. The "Instance" suffix doesn't add any information anyway.

cl-sonarqube-production · 2023-06-29T13:36:59Z

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

91.9% Coverage
0.0% Duplication

bolekk · 2023-06-29T13:43:23Z

core/services/ocr2/delegate.go

-		functionsRelay.FunctionsPlugin,
-	)
+
+	createPluginProvider := func(pluginType functionsRelay.FunctionsPluginType, relayerName string) (types.PluginProvider, error) {


bolekk · 2023-06-29T13:44:26Z

core/services/ocr2/delegate.go

@@ -986,6 +1002,21 @@ func (d *Delegate) newServicesOCR2Functions(
 		ReportingPluginFactory:       nil, // To be set by NewFunctionsServices
 	}

+	s4OracleArgs := libocr2.OCR2OracleArgs{


Do you think we could wrap the creation of this struct in a function too, like you did with the Provider above? We could also consider moving it inside NewFunctionsServices() ...

We won't save much of lines I think.. but loose in readability.

bolekk · 2023-06-29T13:46:01Z

core/services/ocr2/plugins/functions/config/config.go

+
+	pluginConfig := reportingPluginConfigWrapper.Config.S4PluginConfig
+	if pluginConfig == nil {
+		return nil, nil, fmt.Errorf("PluginConfig bytes %x did not contain s4 plugin config", config)


Does it make sense to log all the raw bytes?

I believe this was copied from another plugin. Maybe we don't need to log it, because we always have it on-chain?

* Initializing S4 plugin * Addressed PR feedback

* add an example contract for mercury and log trigger * refactor * refactor * refactor * add a VL contract for log triggered feed lookup upkeep * Change update transmitter balance behaviour (#9624) * Change update transmitter balance behaviour * add test * regen go wrappers and address comments * fix tests * skip coverage for flaky test * remove todo * cleanup merge * regen wrapper * revert ine removal * skip coverage --------- Co-authored-by: FelixFan1992 <fankejin@gmail.com> * Upkeep manager refactor (#9637) * refactor upkeep administrative manager * remove only * fix * fix go build * regen interface * remove only * fix tests * addressed comments * refactor * fix tests * fix error string * fix tests * fix chaincli * Fixes When to Run Live Tests (#9666) * Update AWS action (#9672) * update VL contracts to work with 2.1 (#9673) * update VL contracts to work with 2.1 * fix * Move EVM node pool config (#9656) * Move EVM node pool config * Update after merge * Add test * Update after merge * Cleanup pipeline data references and change back to checkData (#9650) * Cleanup pipeline data references and change back to checkData * fix tests * regen wrappers * regen wrappers * Mercury feeds starts at the first block number not at zero (#9664) * Mercury feeds starts at the first block number not at zero * Update integration test * chainlink-relay => bbcb3a9 * Update core/services/relay/evm/mercury/data_source.go Co-authored-by: Sergei Drugalev <sergei.drugalev@smartcontract.com> * Fix lint * Fix test --------- Co-authored-by: Sergei Drugalev <sergei.drugalev@smartcontract.com> * [Functions] Allowlist support (#9635) * Update mercury telemetry version (#9676) * [Functions] Mark duplicate request ID log as consumed (#9647) Do it to avoid a flood of repetitive error when LogBroadcaster re-submits the same log in a loop. * Refactoring S4 plugin (#9643) * Functions/S4 integration * Fixed whitespace * Removed integration code * Reduce MAX_PERFORM and MAX_CHECK (#9657) * Properly scope logpoller to feedID for mercury (#9680) * [Functions] Use byte array for secrets in RequestData structure (#9677) Co-authored-by: Morgan Kuphal <87319522+KuphJr@users.noreply.github.com> * bump chainlink-env for pod ready improvemeents (#9640) * bump chainlink-env for pod ready improvemeents * bump one more time * function update fix * cleanup checkupkeep and callback (#9651) * cleanup checkupkeep and callback * cleanup * further cleanup * update comment * cleanup * add max revert data size struct * compile * regen wrappers and fix test * fix tests * add test for revert size limit * fix bug * add callback tests * more callback tests * more tests * regen wrappers * fix test * add clarifying comment * fix test * fix chaincli * regen wrappers --------- Co-authored-by: FelixFan1992 <fankejin@gmail.com> * Cleanup TODOs (#9682) * [BCI-1435] Improve EVM Client Readability (#9662) * added type assertion to client * moved chain and client interfaces to common * updated mocks * Added concrete interface of methods in evm client * renamed commontypes to types for headbroadcaster * removed redundent declaration of function in evm client * refactored commontypes to types for txmgr --------- Co-authored-by: Prashant Yadav <34992934+prashantkumar1982@users.noreply.github.com> * Custom configPoller to support multiple Functions plugins (#9648) * custom configPoller to support multiple Functions plugins * removed log * Fixed tests * fixed lint errors * rolled back unneeded lint changes * addressed feedback * fixed error * use imported ConfigPoller interface --------- Co-authored-by: Bolek <1416262+bolekk@users.noreply.github.com> * Use make (#9687) * fix flat fee micro link (#9690) * update url example for mercury (#9691) * Modles.go evm prefix cleanup (#9654) * Modles.go evm prefix cleanup * removed evm prefix for transmitchecker --------- Co-authored-by: Prashant Yadav <34992934+prashantkumar1982@users.noreply.github.com> * [BCF-2299] ocr2 testing (#9684) * Add integration testing OCRV2 helper functions for forwarders * Add forwarders enabled flag to ocr2 smoke test setup func * Add ocr2 with fwds smoke test * Fix forwarders models definitions to match returned json * Fix OCR2TaskJobSpec String method to include ForwardingAllowed field * Change ocr2 fwds smoke test to run for 100 rounds * Improve ocr2_helpers by removing DeployOCRv2Contracts code duplicates * Improve ocr2_helpers by removing CreateOCRv2Jobs code duplicates * Fix fwdrs ocr2 smoke test round val assertion to be of same type * add iLogAutomation geth wrapper (#9665) * Cleanup setConfig on registry (#9692) * Cleanup setConfig * regen wrappers * Allow zero address to be used as proposed admin address (#9695) * Allow zero address to be used as proposed admin address * regen wrappers * fix test * fix reorg upkeep protection to allow bypass by empty blockhash (#9699) * Fix reorg protection to allow bypass via empty blockhash * regen wrappers --------- Co-authored-by: Ryan Hall <RyanRHall@users.noreply.github.com> * Pause checkUpkeep when registry is paused (#9704) * Pause checkUpkeep when registry is paused * regen wrappers * fix test * regen wrappers * cleanup block - conditional trigger naming to be consistent (#9705) * Cleanup block and conditional trigger to be consistent * regen wrappers * regen wrappers * Bump chainlink-starknet (includes latest caigo) (#9137) * bump caigo * Use caigo to encode the point in a canonical way * Update chainlink-starknet * clean up some logging (#9675) * Automation Registrar 2.1 (#9701) * add deployRegistry21 helper * write automation registrar v2.1 * wip * set verify to false for contract verification (#9716) * Remove excess bytes in logging (not necessary to log payload) (#9711) * Functions: connection handler (#9693) * Functions: connection handler prep work * S4 methods * FunctionsConnector tests * Fixed broken test * Addressed PR feedback * Allowlist refactor * Addressed PR feedback * Preparation for Threshold plugin integration with Functions (#9670) * squashing all commits * integrated changes from #9677 * fixed lint error * fixed lint error * fixed proto name conflict * ran protoc * addressed feedback * Addressed feedback * Added decryptionQueueConfig to integration test jobspec * Fix bugs from merge conflict resolution * Initializing S4 plugin (#9725) * Initializing S4 plugin * Addressed PR feedback * Make all error keys in logs consistent (#9718) * clean up evm 20 (#9741) * add a verifiable load contract for log trigger upkeeps * fix encoding issues in registry 2.1 (#9740) * Log event provider (#9583) * log event provider (wip) * fixes 2.1 integration * log event provider + buffer * log event provider: integration test * adding a temp copy from ocr2keepers * increase timeout to stablize test TODO: figure this out w/o timeouts at all * use log struct for check data * move log event provider int test into evm21 * extract 2.1 int tests * aligned default config should fix backfill test instability * fixes and tests * lint * extract logprovider package * extract log packer * stablize test * apply lookback buffer after rate limiting * stablize tests * log struct packing * Registry 2 1 utils contract (#9727) * build AutomationUtils contract and update tests * use report struct as rawReport bytes * update wrappers * add Log struct --------- Co-authored-by: FelixFan1992 <fankejin@gmail.com> * add verifiable load contract for log trigger upkeeps * generate go wrappers * clean up * updates * format * refactor * update * remove build info * format * update * update go script to read results --------- Co-authored-by: Akshay Aggarwal <71980293+infiloop2@users.noreply.github.com> Co-authored-by: Adam Hamrick <adam.hamrick@smartcontract.com> Co-authored-by: george-dorin <120329946+george-dorin@users.noreply.github.com> Co-authored-by: Sam <samsondav@protonmail.com> Co-authored-by: Sergei Drugalev <sergei.drugalev@smartcontract.com> Co-authored-by: Bolek <1416262+bolekk@users.noreply.github.com> Co-authored-by: Andrei Smirnov <andrei.smirnov@smartcontract.com> Co-authored-by: kylesmartin <54827727+kylesmartin@users.noreply.github.com> Co-authored-by: Morgan Kuphal <87319522+KuphJr@users.noreply.github.com> Co-authored-by: Tate <tate.exon@smartcontract.com> Co-authored-by: Chia Yong Kang <chiayongkang@hotmail.com> Co-authored-by: Prashant Yadav <34992934+prashantkumar1982@users.noreply.github.com> Co-authored-by: Simson <simsonraj.easvarasakthi@smartcontract.com> Co-authored-by: ilija42 <57732589+ilija42@users.noreply.github.com> Co-authored-by: Ryan Hall <RyanRHall@users.noreply.github.com> Co-authored-by: Blaž Hrastnik <blaz@mxxn.io> Co-authored-by: Lei <lei.shi@smartcontract.com> Co-authored-by: Amir Y <83904651+amirylm@users.noreply.github.com>

Initializing S4 plugin

ae4170d

pinebit requested review from bolekk, justinkaseman and KuphJr as code owners June 29, 2023 12:15

bolekk requested changes Jun 29, 2023

View reviewed changes

Addressed PR feedback

8e83c8f

bolekk approved these changes Jun 29, 2023

View reviewed changes

bolekk added this pull request to the merge queue Jun 29, 2023

Merged via the queue into develop with commit 8d3cd36 Jun 29, 2023

bolekk deleted the function-s4-init branch June 29, 2023 17:10

FelixFan1992 pushed a commit that referenced this pull request Jul 6, 2023

Initializing S4 plugin (#9725)

a8ef1e2

* Initializing S4 plugin * Addressed PR feedback

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initializing S4 plugin #9725

Initializing S4 plugin #9725

pinebit commented Jun 29, 2023

github-actions bot commented Jun 29, 2023

bolekk Jun 29, 2023

bolekk Jun 29, 2023

pinebit Jun 29, 2023

bolekk Jun 29, 2023

bolekk Jun 29, 2023

cl-sonarqube-production bot commented Jun 29, 2023

bolekk Jun 29, 2023

bolekk Jun 29, 2023

pinebit Jun 30, 2023

bolekk Jun 29, 2023

pinebit Jun 30, 2023

		connectorLogger := conf.Logger.Named("GatewayConnector").With("jobName", conf.Job.PipelineSpec.JobName)
		s4Storage := s4.NewStorage(conf.Logger, *pluginConfig.S4Constraints, s4ORM, utils.NewRealClock())

Initializing S4 plugin #9725

Initializing S4 plugin #9725

Conversation

pinebit commented Jun 29, 2023

github-actions bot commented Jun 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Jun 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment