Skip to content

Remove node.bold.strategy use node.staker.strategy instead#146

Merged
Tristan-Wilson merged 1 commit intomasterfrom
upstream-changes
Sep 11, 2025
Merged

Remove node.bold.strategy use node.staker.strategy instead#146
Tristan-Wilson merged 1 commit intomasterfrom
upstream-changes

Conversation

@KolbyML
Copy link
Member

@KolbyML KolbyML commented Sep 5, 2025

With NIT-3732 OffchainLabs/nitro#3582

nitro-testnode only has to run --node.staker.strategy, not both --node.staker.strategy`` and --node.bold.strategy``

This PR in OffchainLabs/nitro-testnode will make CI pass for OffchainLabs/nitro#3582 docker CI

@cla-bot cla-bot bot added the s label Sep 5, 2025
@KolbyML KolbyML mentioned this pull request Sep 5, 2025
Merged
@Tristan-Wilson
Copy link
Member

Tristan-Wilson commented Sep 9, 2025

The change itself looks fine. Have you read the guide about which branch to use?
https://github.com/OffchainLabs/nitro-testnode?tab=readme-ov-file#branch-selection-guide-for-devs-working-on-nitro-testnode

I think since this depends on an unreleased nitro version it should be on the master branch, but let me know what you think.

Tristan-Wilson
Tristan-Wilson requested changes Sep 9, 2025
View reviewed changes
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Marked for rework because maybe we need to change branch.

@KolbyML KolbyML changed the base branch from release to master September 9, 2025 14:53
@KolbyML
Copy link
Member Author

KolbyML commented Sep 9, 2025

@Tristan-Wilson I think I resolved the concern, ready for another look

@KolbyML KolbyML assigned Tristan-Wilson and unassigned KolbyML Sep 9, 2025
@Tristan-Wilson
Copy link
Member

Tristan-Wilson commented Sep 10, 2025

Right now by changing the base branch in github it is comparing your branch against master, which includes a bunch of stuff from release that isn't in master. It's not to say that we shouldn't merge that stuff at some point, but we don't want to pick it up when we merge this PR which is a fairly targeted change.

I find having a git alias for drawing a graph of the revision history is super helpful for visualizing this. If you add this to your ~/.gitconfig

[alias]
        dag = log --graph --format='format:%C(yellow)%h%C(reset) %C(blue)\"%an\" <%ae>%C(reset) %C(magenta)%cr%C(reset)%C(auto)%d%C(reset)%n%s' --date-order

Then if you run git dag origin/master origin/upstream-changes you get some nice output like this where you can see exactly what's going on.

* 4a579e1 "Kolby Moroz Liebl" <31669092+KolbyML@users.noreply.github.com> 2 days ago (origin/upstream-changes)
| Remove node.bold.strategy use node.staker.strategy instead
*   34f0e1c "Pepper Lebeck-Jobe" <eljobe@gmail.com> 7 days ago (origin/release, origin/HEAD)
|\  Merge pull request #145 from OffchainLabs/ha/traffic-flag
| * e148d9a "Henry" <11198460+godzillaba@users.noreply.github.com> 7 days ago (origin/ha/traffic-flag)
| | syntax fix
| * b7ccba4 "Henry" <11198460+godzillaba@users.noreply.github.com> 7 days ago
|/  add flags for l2 and l3 traffic
*   c7a5862 "Joshua Colvin" <jcolvin@offchainlabs.com> 5 weeks ago
|\  Merge pull request #144 from OffchainLabs/ha/bump-token-bridge-sdk-dep
| * 6a93346 "Henry" <11198460+godzillaba@users.noreply.github.com> 5 weeks ago (origin/ha/bump-token-bridge-sdk-dep)
| | set default token bridge to 1.2.5
| * 3ba7308 "Henry" <11198460+godzillaba@users.noreply.github.com> 5 weeks ago
| | use ha/fix-missing-network-props
| * fed1d70 "TucksonDev" <tucksondev@gmail.com> 5 weeks ago
| | Update token-bridge-contracts tag
| * dc28df9 "Henry" <11198460+godzillaba@users.noreply.github.com> 6 weeks ago
| | fix token transfer script
| * 39869a1 "Henry" <11198460+godzillaba@users.noreply.github.com> 6 weeks ago
| | update default token bridge version
| * cab5714 "Henry" <11198460+godzillaba@users.noreply.github.com> 6 weeks ago
|/  fix null string weth address caused by token bridge sdk dep bump
*   aaa556d "Tristan-Wilson" <87238672+Tristan-Wilson@users.noreply.github.com> 7 weeks ago (origin/master)
|\  Merge pull request #143 from OffchainLabs/release-master-sync

You'll need to rebase your upstream-changes branch onto the current master and force push it.

$ git rebase --onto origin/master upstream-changes~ upstream-changes
$ git push --force origin upstream-changes

Tristan-Wilson
Tristan-Wilson requested changes Sep 10, 2025
View reviewed changes
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Needs rebasing onto master

@KolbyML KolbyML changed the base branch from master to release September 10, 2025 14:51
@KolbyML KolbyML changed the base branch from release to master September 10, 2025 14:58
@KolbyML KolbyML assigned Tristan-Wilson and unassigned KolbyML Sep 10, 2025
@KolbyML
Copy link
Member Author

KolbyML commented Sep 10, 2025

@Tristan-Wilson ready for another look

Tristan-Wilson
Tristan-Wilson approved these changes Sep 11, 2025
View reviewed changes
Copy link
Member

@Tristan-Wilson Tristan-Wilson left a comment

Choose a reason for hiding this comment

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

Nice

@Tristan-Wilson Tristan-Wilson merged commit f293759 into master Sep 11, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tristan-Wilson Tristan-Wilson Tristan-Wilson approved these changes

@joshuacolvin0 joshuacolvin0 Awaiting requested review from joshuacolvin0

@gzeoneth gzeoneth Awaiting requested review from gzeoneth

Assignees

@Tristan-Wilson Tristan-Wilson

Labels

s

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@KolbyML @Tristan-Wilson @joshuacolvin0