-
-
Notifications
You must be signed in to change notification settings - Fork 648
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
Add Arbitrum One and Arbitrum Nova Support #1112
base: master
Are you sure you want to change the base?
Conversation
@@ -1,6 +1,7 @@ | |||
BIN_IMAGE = blockbook-build | |||
DEB_IMAGE = blockbook-build-deb | |||
PACKAGER = $(shell id -u):$(shell id -g) | |||
DOCKER_VERSION = $(shell docker version --format '{{.Client.Version}}') |
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.
Use the same docker version installed on host machine for deb image for safety
@@ -27,7 +28,7 @@ test-all: .bin-image | |||
docker run -t --rm -e PACKAGER=$(PACKAGER) -v "$(CURDIR):/src" --network="host" $(BIN_IMAGE) make test-all ARGS="$(ARGS)" | |||
|
|||
deb-backend-%: .deb-image | |||
docker run -t --rm -e PACKAGER=$(PACKAGER) -v "$(CURDIR):/src" -v "$(CURDIR)/build:/out" $(DEB_IMAGE) /build/build-deb.sh backend $* $(ARGS) | |||
docker run -t --rm -e PACKAGER=$(PACKAGER) -v /var/run/docker.sock:/var/run/docker.sock -v "$(CURDIR):/src" -v "$(CURDIR)/build:/out" $(DEB_IMAGE) /build/build-deb.sh backend $* $(ARGS) |
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.
Mount docker socket to enable the ability to pull docker images from inside a docker container
@@ -9,6 +9,16 @@ RUN apt-get update && \ | |||
apt-get install -y devscripts debhelper make dh-exec zstd && \ | |||
apt-get clean | |||
|
|||
# install docker cli |
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.
Install docker cli to enable binary via docker image
docker container inspect extract > /dev/null 2>&1 && docker rm extract || true | ||
docker create --name extract {{.Backend.DockerImage}} | ||
{{- if eq .Backend.VerificationType "docker"}} | ||
[ "$$(docker inspect --format='{{`{{index .RepoDigests 0}}`}}' {{.Backend.DockerImage}} | sed 's/.*@sha256://')" = "{{.Backend.VerificationSource}}" ] |
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.
docker verification type will validate the hash of the specified docker image
mkdir backend | ||
{{- if ne .Backend.DockerImage "" }} | ||
docker container inspect extract > /dev/null 2>&1 && docker rm extract || true | ||
docker create --name extract {{.Backend.DockerImage}} |
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.
create docker extraction image to allow copying of pre-built contents directly from the official docker image
"docker_image": "offchainlabs/nitro-node:v3.0.3-3ecd01e", | ||
"verification_type": "docker", | ||
"verification_source": "6bfb702e99d66db88658e712ed0d8691abe03797afe436eaab60a6f31e627e0d", | ||
"extract_command": "docker cp extract:/home/user/target backend/target; docker cp extract:/home/user/nitro-legacy backend/nitro-legacy; docker cp extract:/usr/local/bin/nitro backend/nitro", |
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.
example of docker extract command. copy the nitro binary and associated wasm modules
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.
Hello, thanks for the work!
We are currently trying to make the backend run on our dev servers, so we can merge this PR. We are not completely successful yet - running the script /opt/coins/nodes/arbitrum_archive/arbitrum_archive_exec.sh
When we specified our own URL instead of --parent-chain.connection.url http://127.0.0.1:8136
, we are getting the output below:
INFO [10-15|13:40:08.710] Running Arbitrum nitro node revision=v3.0.3-3ecd01e vcs.time=2024-06-26T15:58:10-06:00
INFO [10-15|13:40:08.715] connected to l1 chain l1url=http://our_eth_backend:11016 l1chainid=1
ERROR[10-15|13:40:08.723] on-chain WASM module root did not match with any of the allowed WASM module roots
We will experiment little more and keep you updated. Below, we have found some possible inconsistencies or issues in the code:
"verification_source": "6bfb702e99d66db88658e712ed0d8691abe03797afe436eaab60a6f31e627e0d", | ||
"extract_command": "docker cp extract:/home/user/target backend/target; docker cp extract:/home/user/nitro-legacy backend/nitro-legacy; docker cp extract:/usr/local/bin/nitro backend/nitro", | ||
"exclude_files": [], | ||
"exec_command_template": "/bin/sh -c '{{.Env.BackendInstallPath}}/{{.Coin.Alias}}/arbitrum_exec.sh 2>> {{.Env.BackendDataPath}}/{{.Coin.Alias}}/backend/{{.Coin.Alias}}.log'", |
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.
arbitrum_exec.sh
file does not exist, it should be arbitrum_archive_exec.sh
--init.download-path $DATA_DIR/tmp \ | ||
--auth.jwtsecret $DATA_DIR/jwtsecret \ | ||
--persistent.chain $DATA_DIR \ | ||
--parent-chain.connection.url http://127.0.0.1:8136 \ |
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.
For the archive
version, port 8116
should be needed instead of 8136
|
||
NITRO_BIN=$INSTALL_DIR/nitro | ||
|
||
$NITRO_BIN \ |
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 have found that running this script demands write access to /home/arbitrum
(seems like some data directory for the app?). It is not acceptable for us, is it possible to somehow override this with some option? We would like e.g. the DATA_DIR
location
This PR add support for Arbitrum One and Arbitrum Nova (default and archive versions) and also brings in the ability to use official docker images as a means of acquiring pre-built binaries and associated runtime requirements.
The raitionale here is that most blockchains support official docker images as a first class citizen and this handles all build environment reqs. In the case of arbitrum's nitro node, there was requirement for node + yarn + docker + rust + go among others which was way too much to manage for build from source (https://docs.arbitrum.io/run-arbitrum-node/nitro/build-nitro-locally). There was also no directly provided pre-build binary to leverage.
This seemed safe to me considering docker is already a requirement of blockbook and therefore doesn't add any extra reqs for docker in docker use. It will also make the backend definition requirements much easier where we can just grab the binary from the docker file and call it a day.
Hopefully this change seems reasonable and sane to you. Happy to discuss further. Thanks!