Skip to content

fix: single sequencer#201

Merged
edobry merged 1 commit intomainfrom
edobry/sequencer-fix
Mar 17, 2025
Merged

fix: single sequencer#201
edobry merged 1 commit intomainfrom
edobry/sequencer-fix

Conversation

@edobry
Copy link
Contributor

@edobry edobry commented Mar 17, 2025

This PR fixes a logic bug when provisioning network participants where all participants were mistakenly configured as sequencers.

Follow-up to this PR.

Context

@edobry edobry merged commit 24cb639 into main Mar 17, 2025
8 checks passed
@edobry edobry deleted the edobry/sequencer-fix branch March 17, 2025 21:27
@SozinM
Copy link
Contributor

SozinM commented Mar 19, 2025

Hi! @edobry
Does this line contains any side effects?

if sequencer_enabled:
            sequencer_enabled = False

Because it seems strange to change the variable that isn't used after (but i'm not that familiar with starlark)

Also the reason i used insert(0) is because later we use things like all_el_contexts[0] to choose EL to which batcher would connect. So we must ensure that main EL instance (probably sequencer) is in first position in this list.
It causes side effects when code is changed by someone who doesn't know this quirk (that's why i put this comment in place)

@sigma
Copy link
Collaborator

sigma commented Mar 19, 2025

Hi! @edobry Does this line contains any side effects?

if sequencer_enabled:
            sequencer_enabled = False

Because it seems strange to change the variable that isn't used after (but i'm not that familiar with starlark)

Also the reason i used insert(0) is because later we use things like all_el_contexts[0] to choose EL to which batcher would connect. So we must ensure that main EL instance (probably sequencer) is in first position in this list. It causes side effects when code is changed by someone who doesn't know this quirk (that's why i put this comment in place)

I'm a bit confused. This code is in the participants loop, so the way I'm reading it:

  • flipping sequencer_enabled results in sequencer_enabled being False for all iterations but the 1st one (effectively making the 1st node the sequencer, which I think is aligned with what you say)
  • doing insert(0, ...) repeatedly effectively makes the last participant be the 1st element of the resulting list. Which seems to be the opposite of what that comment stated.

I must be missing something.

As a meta-note, maybe we should refactor this code to return something like

struct(
  sequencer=...
  all_contexts=...
)

So that we don't have to rely on an index-based convention elsewhere. WDYT?

@SozinM
Copy link
Contributor

SozinM commented Mar 19, 2025

Ah, got it!
I didn't saw the loop and assumed it was the end of function, now it become much clearer, thank you :)

sigma pushed a commit that referenced this pull request Jun 17, 2025
🤖 I have created a release *beep* *boop*
---


##
[1.3.0](1.2.0...1.3.0)
(2025-06-17)


### Features

* Add docs section about using contender as l2 transaction source
([#175](#175))
([eea3e1b](eea3e1b))
* add new lint tool
([#197](#197))
([6d078a9](6d078a9))
* add op-faucet component
([#228](#228))
([07fcbfb](07fcbfb))
* add overridable registry
([#253](#253))
([218c277](218c277))
* add support for op-interop-mon
([#325](#325))
([52ed3e6](52ed3e6))
* additional service tx fuzzer
([#192](#192))
([9c5203a](9c5203a))
* adds op-challenger support
([#116](#116))
([6aba40a](6aba40a))
* adds op-proposer support
([#111](#111))
([f062776](f062776))
* adds support for permissionless game, challenger interop support
([#155](#155))
([825b5f5](825b5f5))
* Allow overriding faultGameAbsolutePrestate in op-deployer
([#125](#125))
([2e7b7cd](2e7b7cd))
* alt-da support
([#154](#154))
([0a1593a](0a1593a))
* EL builder sequencer and bootstrap issues
([#178](#178))
([a9e3b2d](a9e3b2d))
* generate op-supervisor dependency set json
([#131](#131))
([464fbba](464fbba))
* **grafana:** add grafana support
([#137](#137))
([3e6f9de](3e6f9de))
* **grafana:** Grizzly compatibility layer
([#280](#280))
([bdd7821](bdd7821))
* improve URL handling
([#174](#174))
([5fb6ecf](5fb6ecf))
* **l2-consensus:** support kona-node consensus clients
([#229](#229))
([2685030](2685030))
* **op-deployer:** optionally receive contracts from enclave artifact
([#203](#203))
([509ef69](509ef69))
* **op-signer:** add op-signer support
([#207](#207))
([40c365b](40c365b))
* **op-supervisor:** add op-supervisor component
([#110](#110))
([99fe41d](99fe41d))
* Per-interop-set supervisor
([#220](#220))
([f9b3519](f9b3519))
* **prometheus:** add prometheus support
([#134](#134))
([e22047a](e22047a))
* **proxyd:** add proxyd support
([#195](#195))
([36444bb](36444bb))
* **rbuilder:** Add op-rbuilder as exectuion layer builder option
([#169](#169))
([ff57cb4](ff57cb4))
* **rbuilder:** Fix rbuilder flag and update builder op-node to latest
format
([#217](#217))
([c993cd0](c993cd0))
* **reproduicibility:** pin ethereum-package dependency
([#135](#135))
([e9eede2](e9eede2))
* **supervisor:** Add support for `kona-supervisor`
([#291](#291))
([d5fb08c](d5fb08c))
* **test:** Unit testing using kurtestosis
([#161](#161))
([1887774](1887774))
* update rollup-boost params
([#177](#177))
([f6ebd93](f6ebd93))


### Bug Fixes

* `kt run .` without --args-file should run without error
([#166](#166))
([d137619](d137619))
* Add depset-config dummy flag for challenger
([#188](#188))
([b6b95c4](b6b95c4))
* Add isthmus activation time to gen2spec tool
([#120](#120))
([9032461](9032461))
* additional fixes for rollup-boost drift
([#163](#163))
([8d6ded4](8d6ded4))
* adjust for new op-deployer
([#250](#250))
([e2de208](e2de208))
* Bump the default version of OP contract deployer
([#146](#146))
([81fa02f](81fa02f))
* **cannon:** update cannon vm type after type 2 was removed
([#247](#247))
([c124cb9](c124cb9))
* **contracts:** use newer cannon
([#235](#235))
([7b49f98](7b49f98))
* Correct handling global params and set the log level of reth
([#183](#183))
([dad910a](dad910a))
* default L1 CL to lodestar
([#273](#273))
([3972491](3972491))
* default optimism_args to empty
([#196](#196))
([a8d31b4](a8d31b4))
* don't truncate names too arbitrarily
([#138](#138))
([edb9ce2](edb9ce2))
* explicitly set op-deployer cache to a reasonable value
([#193](#193))
([0b78140](0b78140))
* Fix challenger <> supervisor interaction
([#230](#230))
([e76bd10](e76bd10))
* Fix k8s pipeline configuration
([#239](#239))
([7cb0d4d](7cb0d4d))
* Fix missing EL/CL with multiple chains
([#227](#227))
([aa14baf](aa14baf))
* Generate all addresses, fix launchers
([#130](#130))
([0bc10d1](0bc10d1))
* **grafana:** use absolute file path
([#176](#176))
([b62804e](b62804e))
* Interop timestamp bringing op-node down
([#298](#298))
([f55f99a](f55f99a))
* nightly tests
([#194](#194))
([631fe94](631fe94))
* **observability:** label improvements
([#185](#185))
([e735f56](e735f56))
* **observability:** network label for supervisor
([#187](#187))
([884f4eb](884f4eb))
* **observability:** remove supervisor network label
([#190](#190))
([bb4a36d](bb4a36d))
* **op-deployer:** allow overriding cannon vm type
([#249](#249))
([2b7c71b](2b7c71b))
* **op-faucet:** set op.network.id label
([#341](#341))
([d36ea59](d36ea59))
* **op-node:** Add depdencency set env var to op-node
([#284](#284))
([3dffb52](3dffb52))
* **proxyd:** uniquify proxyd-config artifact
([#206](#206))
([6a71ee2](6a71ee2))
* **proxyd:** upgrade to v4.14.5
([#232](#232))
([07ee49b](07ee49b))
* **registry:** use published op-faucet image
([#327](#327))
([3407651](3407651))
* Remove dead code in main.star
([#114](#114))
([9e47fd6](9e47fd6))
* remove some UUOC
([#128](#128))
([6ed6c57](6ed6c57))
* removes op-node default param l1.trustrpc
([#140](#140))
([35d4f73](35d4f73))
* repair supervisor->node cnx
([#127](#127))
([6d6c96e](6d6c96e))
* restore semantics of empty strings in input
([#150](#150))
([c32a626](c32a626))
* Revert to pre-multiple-supervisor codebase
([#234](#234))
([b3f3454](b3f3454))
* rollup boost not starting
([#117](#117))
([6eba979](6eba979))
* rollup boost plan errors
([#147](#147))
([069b11d](069b11d))
* set useInterop flag properly
([#122](#122))
([92438b5](92438b5))
* sets op-node sequencer/verifier confs to 2/1
([#141](#141))
([4cfd2da](4cfd2da))
* single sequencer
([#201](#201))
([24cb639](24cb639))
* stop pointing op-node to op-supervisor
([#124](#124))
([ae515ea](ae515ea))
* **superchain:** encode related chains in service names
([#266](#266))
([68d71d7](68d71d7))
* **supervisor:** Add new rollup config set to launcher
([#283](#283))
([d765699](d765699))
* Support multiple sequencers: Add forgotten DA & tx fuzzer input
parsers to the L2 input parser [18/N]
([#304](#304))
([19f32b6](19f32b6))
* Support multiple sequencers: Add missing EL/CL labels [24/N]
([#315](#315))
([b3bd85a](b3bd85a))
* Support multiple sequencers: Add new rbuilder EL launcher; Fix reth
launcher [25/N]
([#316](#316))
([0e7fd19](0e7fd19))
* Support multiple sequencers: Add op-conductor-ops launcher [26/N]
([#320](#320))
([927deb7](927deb7))
* Support multiple sequencers: Challenger labels [41/N]
([#337](#337))
([3fde145](3fde145))
* Support multiple sequencers: Ensure networks with conductors have at
least 2 nodes [33/N]
([#328](#328))
([8bafe6a](8bafe6a))
* Support multiple sequencers: Fix service name for builders [26/N]
([#317](#317))
([989a632](989a632))
* Support multiple sequencers: Kurtosis update & CI clanup [32/N]
([#326](#326))
([147d515](147d515))
* Support multiple sequencers: Make sure network ID label is a string
[15/N]
([#299](#299))
([16ac4cb](16ac4cb))
* Support multiple sequencers: Pass supervisors params to conductor
[29/N]
([#322](#322))
([81038b9](81038b9))
* Support multiple sequencers: Plug rollup boost into conductor [31/N]
([#324](#324))
([448edc6](448edc6))
* Support multiple sequencers: Reorder the launch sequence [28/N]
([#321](#321))
([d0db99c](d0db99c))
* Support multiple sequencers: Run op-conductor-ops [30/N]
([#323](#323))
([e382ed0](e382ed0))
* teku set older version
([#223](#223))
([c79d15e](c79d15e))
* **util:** name artifacts created by write_to_file
([#205](#205))
([44cee08](44cee08))
* wallets.json multi-chain support
([#123](#123))
([288176c](288176c))


### Reverts

* add op-signer support
([#207](#207))
([#225](#225))
([9332474](9332474))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

3 participants

Comments