-
Notifications
You must be signed in to change notification settings - Fork 16
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
Bring local-setup into the V2 era #156
Conversation
This does a few things at once: • Prepares us for a future where there will be multiple projects inside ./docker • Updates the bitcoind instance to the latest one • Updates esplora and electrs
This builds a geth node and deploys contracts
Most of the time this won't be needed (the geth docker resets itself when contracts change), but handy if something goes very wrong.
We build the following services: • geth (ethereum) with contracts deployed • geth block explorer (localhost:8095) • bitcoind (bitcoin) • bitcoin api • bitcoin block explorer (localhost:8096)
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.
What do we plan to do about the failed CI jobs? Are they still relevant or should we remove them?
RUN apk add --update --no-cache bash git geth make gcc g++ && \ | ||
npm install --global yarn --force | ||
|
||
# There's a transitive git problem that this fixes |
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.
Is it this? Could be adding some reference in case the root cause is resolved in the future.
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.
yeah!
.gitmodules
Outdated
url = https://github.com/keep-network/keep-core.git | ||
[submodule "solidity-contracts"] | ||
path = solidity-contracts | ||
url = https://github.com/threshold-network/solidity-contracts |
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.
Not in this PR but we could think about introducing some structure for submodules, for example:
submodules
keep-network
keep-core
tbtc-v2
threshold-network
solidity-contracts
With the current setup, the solidity-contracts
directory is confusing without looking at .gitmodules
. What solidity contracts are we talking about? Are those tBTC or Keep contracts?
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.
went ahead and did it b195086
Dockerfile.geth
Outdated
|
||
COPY ./solidity-contracts /solidity-contracts | ||
WORKDIR /solidity-contracts | ||
RUN git init |
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.
Do we need togit init
? I think we should be able to remove it from here, as well as line 37 and 57.
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.
i forget which repository needs to register as a git repo for the later commands to work, but it's at least one of them
having the projects as submodules means that rather than having a .git/
folder, they have .git
files that point back at the parent directory, and that parent directory doesn't exist when we're copying
I can definitely go back and see which ones we can safely remove!
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.
oh! i think the git init
thing was required when i was actually running keep-core's install.sh
. Since I'm not anymore (it's unpacked into the docker file and entrypoint), I can remove all of them 🎉
good catch! facc694
RUN git init | ||
WORKDIR /tbtc-v2/solidity | ||
RUN yarn install --network-timeout 1000000 && \ | ||
yarn build || true |
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.
Why || true
? Should we continue if yarn build
failed?
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.
I'll add a comment, but for whatever reason the first yarn build
almost always fails and the second yarn build always succeeds. happened both in docker and locally for me
rather than go down that rabbit hole, i just have docker try to build it twice
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.
Added a comment: c42e76a
|
||
RUN git checkout a33e97e1a1fc63fa9c20a116bb92579bbf43b254 | ||
|
||
RUN git checkout 20e42862a57f0d45ce48a9956a361e7193b9e97d |
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.
What is specific about this commit? Is it the most recent tag version or some "last working" version? Can we add a comment? Same question about other Dockerfile
s.
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.
it's the most recent tag - i'll see if i can just check the tag out directly or add a comment
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.
A couple of notes:
Once I re-run, the build have passed.
I run |
/ethereum/.data/ | ||
/bitcoin/.data/ | ||
/docker/bitcoin/.data/ | ||
/docker/ethereum/.data/ |
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 should hold ethereum explorer cache, right? I'm asking because I don't have this folder, despite successfully running the docker container (likely related to seeing b-explorer | Error trying to update latest Transactions cache.
in the container logs).
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 transaction cache stuff is expected - i'll call that out in the PR description
./docker/ethereum/.data/
is where we're storing ethereum chain data. docker-compose.yml
is handling the volume work for us:
geth:
container_name: geth
build:
context: .
dockerfile: Dockerfile.geth
volumes:
- ./docker/geth/.data:/ethereum/data
This helps organize code a little better and also provides context to what "solidity-contracts" is
These were necessary back when I was running keep-core's install.sh directly (which assumes it's being run in a git repo). Over time, I replicated the functionality in install.sh and split it up into the dockerfile and entrypoint for efficiency. We want as much as possible to live at build-time!
When we run it a single time, we get: ``` #0 62.31 Error HH501: Couldn't download compiler version 0.8.17+commit.8df45f5f. Please check your internet connection and try again. #0 62.31 HardhatError: HH501: Couldn't download compiler version 0.8.17+commit.8df45f5f. Please check your internet connection and try again. #0 62.31 at /tbtc-v2/solidity/node_modules/hardhat/src/internal/solidity/compiler/downloader.ts:175:15 #0 62.31 at async CompilerDownloader.downloadCompiler (/tbtc-v2/solidity/node_modules/hardhat/src/internal/solidity/compiler/downloader.ts:150:5) #0 62.31 at async SimpleTaskDefinition.action (/tbtc-v2/solidity/node_modules/hardhat/src/builtin-tasks/compile.ts:585:9) #0 62.31 at async Environment._runTaskDefinition (/tbtc-v2/solidity/node_modules/hardhat/src/internal/core/runtime-environment.ts:308:14) #0 62.31 at async Environment.run (/tbtc-v2/solidity/node_modules/hardhat/src/internal/core/runtime-environment.ts:156:14) #0 62.31 at async SimpleTaskDefinition.action (/tbtc-v2/solidity/node_modules/hardhat/src/builtin-tasks/compile.ts:678:36) #0 62.31 at async Environment._runTaskDefinition (/tbtc-v2/solidity/node_modules/hardhat/src/internal/core/runtime-environment.ts:308:14) #0 62.31 at async Environment.run (/tbtc-v2/solidity/node_modules/hardhat/src/internal/core/runtime-environment.ts:156:14) #0 62.31 at async Environment._runTaskDefinition (/tbtc-v2/solidity/node_modules/hardhat/src/internal/core/runtime-environment.ts:308:14) #0 62.31 at async OverriddenTaskDefinition._action (/tbtc-v2/solidity/node_modules/@openzeppelin/hardhat-upgrades/src/index.ts:77:33) #0 62.31 #0 62.31 Caused by: HeadersTimeoutError: Headers Timeout Error #0 62.31 at Timeout.onParserTimeout [as _onTimeout] (/tbtc-v2/solidity/node_modules/undici/lib/client.js:894:28) #0 62.31 at listOnTimeout (node:internal/timers:561:11) #0 62.31 at processTimers (node:internal/timers:502:7) #0 62.48 error Command failed with exit code 1. ``` But it always works on the second time. There is probably a way to fix this "better", but for now just running it twice (ignoring the error with `|| true`) works.
Blockstream's electrs doesn't use git tags, and we don't want the docker builds to change behavior based on when they're built, so we need to check out code at a specific point. We choose the most up-to-date branch in the repo, which happens to be `202305-fix-mempool-batching`. Unclear as to why these branches don't make their way back into the main branch. Unclear as to what the main branch is.
💯
Added!
I see this too sometimes, totally unclear as to why; it goes away if i just rebuild
The ethereum block explorer is definitely inconsistent. It works for me sometimes. I'm down to take it out or leave it in. I also might try to adopt the code one weekend and make it actually work
this also happened to me a handful of times while building; very inconsistently though |
They're out of date and failing. If we want to build similar ones in the future, we can!
Nuking them: 8c71a0e |
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.
I've tested the setup again after the changes, this time I got Error: Lock is not acquired/owned by you
once during building, but it worked after the re-run.
Apart from that I've noticed that we don't use some (or maybe all) of the files in the configs
folder. Also, the prettier-related configs are out of date, I think they can be removed if we don't have package.json
?
Also I'm not sure if we need the bitcoind-wallet
folder.
cd $WORKDIR | ||
|
||
# Copy all config files to the right relay directory. | ||
cp -R configs/relays/. relays/maintainer/maintainer/config/ |
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.
I think we can remove the files in configs/relays
, this looks like the only place they were used in.
printf "${LOG_START}Copying config files...${LOG_END}" | ||
|
||
# Copy all config files to the right keep-core directory. | ||
cp -R configs/keep-core/. keep-core/configs/ |
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.
I think we can remove the files in configs/keep-core
, this looks like the only place they were used in.
printf "${LOG_START}Copying config files...${LOG_END}" | ||
|
||
# Copy all config files to the right keep-ecdsa directory. | ||
cp -R configs/keep-ecdsa/. keep-ecdsa/configs/ |
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.
I think we can remove the files in configs/keep-ecdsa
, this looks like the only place they were used in.
refs:keep-network/keep-core#3617
Features
It's pretty tough to run all of the moving pieces of v2, so this PR is the first half of being able to build everything in docker!
Implemented:
•
geth
with all the contracts deployed, exposed at the default port (localhost:8545)• a local ethereum block explorer (localhost:8095)
•
bitcoind
exposed at the default port (localhost:18332)• this interacts out of the box with our
bitcoind-wallet
•
electrs
exposed at the default port (localhost:3002)• a local bitcoin block explorer (localhost:8094)
The bitcoin block explorer works great:
the ethereum block explorer sometimes works:
but sometimes does not:
Next step is getting the UI hooked up!
Testing
clone the repo
then
• You should be able to see block explorers running at http://localhost:8094 and http://localhost:8095
• The log output should be talking about mining blocks and sometimes deploying contracts
• If you see a message that says "Contract Deployment completed!" then deployment is done. If you tear down the dockers (ctrl+c) and then bring them back up (
docker-compose up
), then it should skip deployment.• The ethereum block explorer has a lot of noisy error logs but is useful enough to keep in its current state imo.
Tagging @michalinacienciala @nkuba @pdyraga for review. I'm not a docker expert so any help reformatting this stuff or on better idioms would be appreciated!