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

ci-builder: update geth version #3714

Merged
merged 5 commits into from
Oct 21, 2022
Merged

ci-builder: update geth version #3714

merged 5 commits into from
Oct 21, 2022

Conversation

tynes
Copy link
Contributor

@tynes tynes commented Oct 17, 2022

Updates the version of abigen that is used in the ci-builder so that it produces bindings that do not parse JSON every time that an instance of the binding is created.

The change in geth is here: ethereum/go-ethereum#25574

Fixes the additional diff found here: https://github.com/ethereum-optimism/optimism/pull/3680/files

Right now we do not have a guarantee that local devs use a specific version of abigen. After this is merged, all devs would need to use v1.10.25. Devs could use higher versions but risk running into problems if there are more changes to abigen in the future.

@changeset-bot
Copy link

changeset-bot bot commented Oct 17, 2022

🦋 Changeset detected

Latest commit: 040ea97

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/ci-builder Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

This is a cool feature, but depends on a very recent version of geth & abigen. Getting this merged seems worth it, but it needs to be documented in where we list setup requirements & communicated to the whole team.

@tynes
Copy link
Contributor Author

tynes commented Oct 17, 2022

This is a cool feature, but depends on a very recent version of geth & abigen. Getting this merged seems worth it, but it needs to be documented in where we list setup requirements & communicated to the whole team.

Good call, I was trying to fix #3680

I can add something to the docs

@smartcontracts
Copy link
Contributor

We really need to add this to the docs. #3343 talks about this issue too.

Updates the version of `abigen` that is used in the
`ci-builder` so that it produces bindings that do not
parse JSON every time that an instance of the binding
is created.

The change in geth is here: ethereum/go-ethereum#25574

Fixes the additional diff found here: https://github.com/ethereum-optimism/optimism/pull/3680/files

Right now we do not have a guarantee that local devs use
a specific version of `abigen`. After this is merged,
all devs would need to use `v1.10.25`. Devs could use higher
versions but risk running into problems if there are more
changes to `abigen` in the future.
@tynes
Copy link
Contributor Author

tynes commented Oct 20, 2022

I documented how to install abigen as well as the specific version that we should use after this PR is merged and the new ci-builder image is released

Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

lgtm

@tynes tynes requested a review from maurelian October 21, 2022 15:47
@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot merged commit 3387c28 into develop Oct 21, 2022
@mergify mergify bot deleted the ops/update branch October 21, 2022 19:11
@mergify mergify bot removed the on-merge-train label Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ops Area: ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants