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

Update simulated geth client wrapper & LogPoller tests #13204

Merged

Conversation

reductionista
Copy link
Contributor

@reductionista reductionista commented May 15, 2024

In this PR:

  1. Update SimulatedBackendClient to wrap simulated.Backend & simulated.Client ( instead of deprecated backends.SimulatedBackendClient )

  2. Add SetActiveClient() to SimulatedBackendClient to simulate switching from one rpc server to another while running.

  3. Add RegisterHeadByNumberCallback to SimulatedBackendCilent, to register a custom callback function to be called each timeHeadByNumber() is called.

  4. Add to SimulateBackendClient ability to run in "optimism mode", which behaves slightly different from geth: returns success rather than "block not found" if FilterLogs() is called with an invalid or unknown block hash.

  5. Update BackupPoller test to use the previous 3 features to simulate failing over.to an out-of-sync optimism rpc server during a race condition between pulling a particular block and its corresponding logs. (This is exactly the case which motivated the creation of Backup LogPoller.) This avoids the need to use rawdb, which is no longer accessible in geth.

  6. Replace markBlockAsFinalized() and markBlockAsFinalizedByHash() in LogPoller tests with finalizeThroughBlock().

  7. Refactor logic in LogPoller tests previously using markBlockAsFinalized(ByHash) to make use of finalizedThroughBlock() instead.

  8. Update some tests for new block timestamp behavior in Simulated Geth: automatic time increment between simulated blocks is 1ns now instead of 1s, and Client.AdjustTime() now automatically commits the current block after adjusting its timestamp

  9. Update rest of tests in LogPoller and other packages to use new data types associated with (1.), instead of deprecated types

@reductionista reductionista force-pushed the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch from d5f0bb2 to 9be1d30 Compare May 15, 2024 03:28
Copy link
Contributor

I see you updated files related to core. Please run pnpm changeset in the root directory to add a changeset as well as in the text include at least one of the following tags:

  • #added For any new functionality added.
  • #breaking_change For any functionality that requires manual action for the node to boot.
  • #bugfix For bug fixes.
  • #changed For any change to the existing functionality.
  • #db_update For any feature that introduces updates to database schema.
  • #deprecation_notice For any upcoming deprecation functionality.
  • #internal For changesets that need to be excluded from the final changelog.
  • #nops For any feature that is NOP facing and needs to be in the official Release Notes for the release.
  • #removed For any functionality/config that is removed.
  • #updated For any functionality that is updated.
  • #wip For any change that is not ready yet and external communication about it should be held off till it is feature complete.

@reductionista reductionista changed the title Bcf 3134 refactor backup logpoller test for geth bump Update simulated geth client wrapper & LogPoller tests May 15, 2024
@reductionista reductionista force-pushed the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch from 74c13d9 to 81aa9b7 Compare May 15, 2024 03:42
Add RegisterHeadNumberCallback() to SimulatedBackendClient, so we can
trigger an rpc failover event just after reading a particular block, but
before the logs get read for that block. This is the race condition that
can happen on optimism chain which BackupPoller was designed to address
@reductionista reductionista force-pushed the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch from be613b7 to aec7ffe Compare May 24, 2024 02:43
@reductionista reductionista marked this pull request as ready for review May 24, 2024 03:40
@reductionista reductionista requested review from a team as code owners May 24, 2024 03:40
@reductionista reductionista force-pushed the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch from a5a60a5 to fcae628 Compare May 29, 2024 04:27
2 Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

scratch file?

@reductionista reductionista force-pushed the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch from fcae628 to 1a6e9ff Compare May 29, 2024 18:01
@reductionista
Copy link
Contributor Author

Present status:

  • All tests in LogPoller package passing

  • All deprecated types have been updated (in all packages), Lint is now passing

  • Seeing many errors in the Core CI tests still, but nearly all of them are one of 3 things, which seem very related and possibly have the same root cause:
    no contract code at given address
    Execution reverted
    `Execution reverted: Target is not a contract"

  • One weird error in a core test, that happens in CI but passes locally:
    --- FAIL: TestEVMForwarderPresenter_RenderTable (0.00s)
    forwarders_commands_test.go:48:
    Error Trace: /home/runner/work/chainlink/chainlink/core/cmd/forwarders_commands_test.go:48
    Error: "\n------------------------------------------------------\nID: \nAddress: 0x4Fc0FeeD7E3F8d9bb6758d3084aa0ff6cFB5C486\nChain ID: 4\nCreated At: 2024-05-29T04:33:50Z\n------------------------------------------------------\n" does not contain "1"

  • For some reason, this error causes the Clean Go Tidy & Generate CI check to fail:
    cp: cannot create directory '../../operator_ui/..//core/web/assets': No such file or directory

I think the last one was what prompted me initially to run make generate locally, but I guess that didn't fix it.

I'm not sure how many of these errors were already in the branch I'm trying to merge into, but seems like it might be about time to merge it and rebase that branch?

…aside from the // error at the end that's in CI as well.
@jmank88 jmank88 merged commit 107d9f8 into bump-geth Jun 4, 2024
113 of 116 checks passed
@jmank88 jmank88 deleted the BCF-3134-refactor-backup-logpoller-test-for-geth-bump branch June 4, 2024 23:14
github-merge-queue bot pushed a commit that referenced this pull request Nov 12, 2024
* bump geth v1.13.14; rm hack; fix simulated backend; run make generate

* Update simulated geth client wrapper & LogPoller tests (#13204)

* Re-run make generate, fix fluxmonitorv2 & ocr2keeper tests

* Update SimulatedBackendClient to wrap simulated.Backend & simulated.Client

( instead of deprecated backends.SimulatedBackendClient )

* Update LogPoller helper

* Add support for switching rpc clients in simulated geth

* Fix TestLogPoller_BackupPollAndSaveLogsSkippingLogsThatAreTooOld

This test relied on markBlockFinalized() which has been replaced
with finalizedBlocksThrough(). Just needed some slight adjustments
to keep testing the same thing.

* Fix TestLogPoller_ReorgDeeperThanFinality

* Fix Test_PollAndSavePersistsFinalityInBlocks

* Fix Test_PollAndQueryFinalizedBlocks

* update listener_v2_log_listener_test

* Fix chainreader & config poller tests

* Update keeper integration tests

* Re-run make generate

* Update BackupLogPoller test

Add RegisterHeadNumberCallback() to SimulatedBackendClient, so we can
trigger an rpc failover event just after reading a particular block, but
before the logs get read for that block. This is the race condition that
can happen on optimism chain which BackupPoller was designed to address

* .

* Update TestLogPoller_PollAndSaveLogsDeepReorg

* Not sure how this was working before

Presumably, the older verison of simulated.Backend would fill in fake timestamps
instead of real ones?

* Update TestLogPoller_PollAndSaveLogs

* Update TestLogPoller_Blocktimestamps

The new Simulated Geth made two changes which affected this test
1. The automatic time interval added to each new block is now 1ns instead of 1s
2. AdjustTime() now automatically calls Commit() so it no longer needs to be called aftewards

* Update TestLogPoller_BackupPollAndSaveLogsWithPollerNotWorking

* Address PR review comments

- Consolidate go-ethereum imports
- Remove extra geth-wrapper changes

* Update types in vrf tests

* Update keepers, fluxmointor, transmitter,ocr test types

* re-generate KeystoneForwarder

* Replace optimismMode with chainFamily enum

* Update more types GenesisAlloc, GenesisAccount, llo

* Fix some more compilation errors (fluxmonitor2, vrf, ocr2, functions)

* Fix lint errors, remove unused logpoller test definitions

* Run go-generate again on KeystoneForwarder

* Re-run "make generate" one more time, this time without any failures aside from the // error at the end that's in CI as well.

* Re-generate generation version db for keystone

* cleanup

* make generate

* cleanup

* race-free simulated backend commits

* race free commits

* fix tests by adding commits

* bump geth to 1.13.15

* fix more races

* Remove inconsistent named param from return signature

* insert temporary skips

* make generate

* skip more tests

* skip more tests; fill zeroed timestamp on insert

* fix race for commit

* core/services/ocr2/plugins/ocr2keeper/evmregistry/v21/logprovider: run log poller

* re-enable some tests

* consolidate finalization helpers

* fix some tests with real timestamps

* simplify

* core/services/vrf/solidity_cross_tests: raise gas estimate upper bound in TestMeasureRandomnessRequestGasCost

* cleanup and patch for geth bug

* new geth error; vrf test fixes

* fix ccip tests

* fix clo tests

* more fixes

* fix test var name

* fix compile error

* upgrade to 14.7

* fix lint

* lint issues

* make generate

* fix more tests

* skip failing tests

* skip tests; linter issues

* fix race & lint issues

* BCFR-1047 Fix HT Tests (#14807)

* Simulated defaults (#14986)

* Simulated defaults

* Direct request works

* Fix config tests

* AsyncEthTx

* OCR + FM

* Keepers

* Functions

* Bunch of keeper and VRF tests

* More vrf tests

* Cleanup

* Fix a race

* core/chains/evm/txmgr: fix TestEthBroadcaster_ProcessUnstartedEthTxs_OptimisticLockingOnEthTx

* core/gethwrappers: regenerate

* bump geth to 1.14.11 (#15041)

* bump geth to 1.14.11

* remove AdjustTime hacks

* Try replace in integration tests

* Copy replace around

* Preserve fork behaviour

* generate

* tidy

* Work around geth, fix timestamp test

* Fix docstring

* Fix reader test

* Linter

* Re-enable CCIP deployment tests

* Mod + lint

* Remove cltest dependency which invokes pgtest init

---------

Co-authored-by: connorwstein <[email protected]>

* Another cltest remove

* Go mod tidy

* Regen lint and fix LLO test

* Enable some more tests

* core: prefer simulated backend interface in order to wrap with syncBackend

* fix race: cleanup int conversion

* lint

* lint & fix

* lint

* lint

* unskip and fix

* Fix TestIntegration_OCR2_plugins

* Fix more OCR tests

* update t.Skip() messages

* update changeset

* Resurrect CCIP tests

* Lint

* replaces

* lint

---------

Co-authored-by: Domino Valdano <[email protected]>
Co-authored-by: Domino Valdano <[email protected]>
Co-authored-by: AnieeG <[email protected]>
Co-authored-by: Dmytro Haidashenko <[email protected]>
Co-authored-by: Connor Stein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants