[Functions] Allowlist support #9635

bolekk · 2023-06-16T22:21:15Z

No description provided.

github-actions · 2023-06-16T22:21:42Z

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

KuphJr · 2023-06-16T22:26:56Z

core/services/gateway/handlers/functions/allowlist.go

+	"github.com/smartcontractkit/chainlink/v2/core/services/job"
+)
+
+// OnchainAllowlist maintains an allowlist of addresses that if fetched from the blockchain (EVM-only).


What does this comment line mean?

"if" -> "is"

KuphJr · 2023-06-16T22:35:53Z

core/services/gateway/handlers/functions/allowlist.go

+	rawResponse, err := a.client.CallContract(context.Background(), ethereum.CallMsg{
+		To:   &a.contractAddress,
+		Data: a.methodSignature,
+	}, big.NewInt(-1))


Should the block height be configurable instead of just using -1 in order to ensure support for chains w/ different finalities?

Good point, I'll add that option.

KuphJr · 2023-06-16T22:39:17Z

core/services/gateway/handlers/functions/allowlist.go

+}
+
+func (a *onchainAllowlist) UpdateFromChain() error {
+	rawResponse, err := a.client.CallContract(context.Background(), ethereum.CallMsg{


Does the core node not have any simplified utility functions which allows you to specify the function ABI and return type ABI as strings to perform the contract call & decode the response? This seems like it may be useful elsewhere.

I don't think there's a generic utility function but your comment made me realize that ABI types for all contract functions are auto-generated for me by gethwrappers. I'll use those to simplify my code.

bolekk · 2023-06-20T00:57:17Z

I think I found a small bug in evm Client: #9652
This PR depends on the fix.

bolekk · 2023-06-20T01:48:15Z

core/chains/evm/client/client.go

@@ -256,7 +256,7 @@ func (client *client) BlockByHash(ctx context.Context, hash common.Hash) (*types
 }

 func (client *client) LatestBlockHeight(ctx context.Context) (*big.Int, error) {
-	var height *big.Int
+	var height big.Int


pinebit

LGTM

KuphJr

LGTM

pinebit · 2023-06-21T10:11:25Z

core/services/gateway/handlers/functions/handler.functions.go

@@ -48,10 +104,21 @@ func (h *functionsHandler) HandleNodeMessage(ctx context.Context, msg *api.Messa
 	return nil
 }

-func (h *functionsHandler) Start(context.Context) error {
+func (h *functionsHandler) Start(ctx context.Context) error {
+	if h.allowlist != nil {


Should not this handler implement StartStopOnce to prevent such the checks?

Allowlist can be nil if AllowlistCheckEnabled is set to false. But good point about StertSTopOnce, I'll add it (maybe in a separate PR).

pinebit · 2023-06-21T10:12:10Z

core/services/gateway/handlers/functions/handler.functions.go

 	return nil
 }

 func (h *functionsHandler) Close() error {
+	h.serviceCancel()
+	h.shutdownWaitGroup.Wait()


The second call to Close will block forever. This is why I mentioned StartStopOnce util.

pinebit

Added comments about StartStopOnce

cl-sonarqube-production · 2023-06-21T14:43:56Z

SonarQube Quality Gate

61.1% Coverage on New Code (is less than 80%)

See analysis details on SonarQube

* 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>

bolekk requested review from pinebit and KuphJr June 16, 2023 22:21

KuphJr reviewed Jun 16, 2023

View reviewed changes

bolekk force-pushed the feature/functions-handler branch from f06c6a9 to f010706 Compare June 20, 2023 00:56

bolekk marked this pull request as draft June 20, 2023 00:56

bolekk marked this pull request as ready for review June 20, 2023 01:29

bolekk force-pushed the feature/functions-handler branch from f010706 to df6f68d Compare June 20, 2023 01:47

bolekk requested review from samsondav and prashantkumar1982 as code owners June 20, 2023 01:47

bolekk commented Jun 20, 2023

View reviewed changes

bolekk force-pushed the feature/functions-handler branch from df6f68d to dab584b Compare June 20, 2023 02:57

pinebit previously approved these changes Jun 20, 2023

View reviewed changes

bolekk enabled auto-merge June 20, 2023 17:06

[Functions] Allowlist support

a95408c

bolekk dismissed pinebit’s stale review via a95408c June 20, 2023 17:42

bolekk force-pushed the feature/functions-handler branch from eedf07c to a95408c Compare June 20, 2023 17:42

KuphJr approved these changes Jun 20, 2023

View reviewed changes

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

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023

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

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023

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

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023

Merge branch 'develop' into feature/functions-handler

4cc1ea9

bolekk enabled auto-merge June 20, 2023 20:40

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

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 20, 2023

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

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 21, 2023

Merge branch 'develop' into feature/functions-handler

6efd946

bolekk enabled auto-merge June 21, 2023 02:38

bolekk disabled auto-merge June 21, 2023 03:56

bolekk enabled auto-merge June 21, 2023 03:56

Merge branch 'develop' into feature/functions-handler

9268d1e

bolekk disabled auto-merge June 21, 2023 06:10

bolekk enabled auto-merge June 21, 2023 06:10

pinebit reviewed Jun 21, 2023

View reviewed changes

Merge branch 'develop' into feature/functions-handler

f939626

bolekk disabled auto-merge June 21, 2023 14:40

bolekk enabled auto-merge June 21, 2023 14:40

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

Merged via the queue into develop with commit ec9e54c Jun 21, 2023
94 of 95 checks passed

bolekk deleted the feature/functions-handler branch June 21, 2023 15:34

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

[Functions] Allowlist support (#9635)

ac85db1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Functions] Allowlist support #9635

[Functions] Allowlist support #9635

bolekk commented Jun 16, 2023

github-actions bot commented Jun 16, 2023

KuphJr Jun 16, 2023

bolekk Jun 19, 2023

KuphJr Jun 16, 2023

bolekk Jun 19, 2023

KuphJr Jun 16, 2023

bolekk Jun 19, 2023

bolekk commented Jun 20, 2023 •

edited

Loading

bolekk Jun 20, 2023

pinebit left a comment

KuphJr left a comment

pinebit Jun 21, 2023

bolekk Jun 21, 2023

pinebit Jun 21, 2023

pinebit left a comment

cl-sonarqube-production bot commented Jun 21, 2023

[Functions] Allowlist support #9635

[Functions] Allowlist support #9635

Conversation

bolekk commented Jun 16, 2023

github-actions bot commented Jun 16, 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

Choose a reason for hiding this comment

bolekk commented Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

pinebit left a comment

Choose a reason for hiding this comment

KuphJr left a 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

pinebit left a comment

Choose a reason for hiding this comment

cl-sonarqube-production bot commented Jun 21, 2023

bolekk commented Jun 20, 2023 •

edited

Loading