-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
e7a27e6
to
fcfd8b8
Compare
fcfd8b8
to
24336de
Compare
I'd give this a rebase off latest |
Will try again, but I'm pretty sure that this is at least from this week. EDIT: The L2Output submitter contracts were not updated. |
24336de
to
f648a12
Compare
--type l1block \ | ||
--out ./l1block/l1_block_info_raw.go | ||
cat $(temp)/combined.json | jq -r ".contracts | with_entries(select(.key | match(\"L1Block\")))[] | .\"bin-runtime\"" > bin/l1block_deployed.hex | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is currently nothing enforcing the version of solc
being used which could result in non-determinism between different machines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what foundry uses to manage versioning: https://github.com/roynalnaruto/svm-rs
Is there anyway that we can get the combined json from forge build
? That would abstract away the solc
version management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have pragmas on the contracts? I had to switch versions of solc
this morning to get this to work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we previously did do something where we would use the artifacts
from hardhat. My big worry with that is if it doesn't get rebuilt prior to running this so I prefer building from source here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pragmas must be exact matches for this to be safe, it looks like they generally are except for the output oracle:
pragma solidity >=0.8.10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also a --solc
flag for abigen
that lets you pass a path to solc
and it'll compile + build the bindings. Perhaps we allow for a configurable solc
path with just using solc
as a default, this way anybody can build and we aren't opinionated about how the user manages the version of solc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I'll switch this just 0.8.10
and create an issue to look into this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Longer term the solution should be something like forge bind go
where foundry manages the bindings being up to date
opnode/contracts/Makefile
Outdated
cat $(temp)/combined.json | jq -r ".contracts | with_entries(select(.key | match(\"L1Block\")))[] | .\"bin-runtime\"" > bin/l1block_deployed.hex | ||
|
||
|
||
bindings-optimism-oracle: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This name makes it sound like its meant to build the output oracle but it builds the portal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
f648a12
to
b996fe3
Compare
This is rather than hardcoding the contract value. It slows down the e2e tests (by having to wait 1 L1 block for contracts to be ready), but it is more accurate to what occurs (and is now required as we initialize actions).
b996fe3
to
89dc34c
Compare
This is rather than hardcoding the contract bytecode. It slows down the e2e
tests (by having to wait 1 L1 block for contracts to be ready), but it
is more accurate to what occurs (and is now required as we initialize
actions).
It also uses the new contracts. No CI for them yet.