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

Bring local-setup into the V2 era #156

Closed
wants to merge 15 commits into from
Closed

Bring local-setup into the V2 era #156

wants to merge 15 commits into from

Conversation

beaurancourt
Copy link

@beaurancourt beaurancourt commented Jun 22, 2023

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:

image

image

the ethereum block explorer sometimes works:

image
image
image
image

but sometimes does not:
image


Next step is getting the UI hooked up!

Testing

clone the repo

git submodule update --init --recursive

then

docker-compose build
docker-compose up

• 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!

Beau Shinkle added 9 commits July 6, 2023 10:41
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)
Copy link
Member

@pdyraga pdyraga left a 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
Copy link
Member

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.

Copy link
Author

Choose a reason for hiding this comment

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

yeah!

aede19d

.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
Copy link
Member

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?

Copy link
Author

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
Copy link
Member

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.

Copy link
Author

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!

Copy link
Author

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
Copy link
Member

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?

Copy link
Author

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

Copy link
Author

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
Copy link
Member

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 Dockerfiles.

Copy link
Author

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

Copy link
Author

Choose a reason for hiding this comment

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

@michalinacienciala
Copy link
Contributor

A couple of notes:

  1. Not necessarily in this PR, but we should remember about updating README.
  2. As I didn't work with submodules in a long time, I didn't remember initially to execute git submodule init and git submodule update in the root folder. We could add those commands to the Testing section in the PR description.
  3. My first run of docker-compose build after getting submodules sorted out have failed, with the following error:
=> [stage-1 22/45] RUN yarn link @threshold-network/solidity-contracts                                                                                                                                                                                               1.0s
 => ERROR [stage-1 23/45] RUN yarn build                                                                                                                                                                                                                            128.9s
------
 > [stage-1 23/45] RUN yarn build:
#29 0.688 yarn run v1.22.19
#29 0.888 $ hardhat compile
#29 10.80 Solidity 0.8.17 is not fully supported yet. You can still use Hardhat, but some features, like stack traces, might not work correctly.
#29 10.80 
#29 10.80 Learn more at https://hardhat.org/hardhat-runner/docs/reference/solidity-support
#29 10.80 
#29 15.92 Downloading compiler 0.8.17
#29 77.31 Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.
#29 77.31 --> @keep-network/sortition-pools/contracts/Branch.sol
[...]
#29 77.31 
#29 77.31 Warning: Contract code size is 24991 bytes and exceeds 24576 bytes (a limit introduced in Spurious Dragon). This contract may not be deployable on Mainnet. Consider enabling the optimizer (with a low "runs" value!), turning off revert strings, or using libraries.
#29 77.31   --> contracts/test/RandomBeaconStub.sol:11:1:
#29 77.31    |
#29 77.31 11 | contract RandomBeaconStub is RandomBeacon {
#29 77.31    | ^ (Relevant source part starts here and spans across multiple lines).
#29 77.31 
#29 77.31 
#29 128.7 An unexpected error occurred:
#29 128.7 
#29 128.7 Error: Lock is already released
#29 128.7     at /keep-core/solidity/random-beacon/node_modules/proper-lockfile/lib/lockfile.js:257:60
#29 128.7     at /keep-core/solidity/random-beacon/node_modules/proper-lockfile/lib/adapter.js:39:9
#29 128.7     at new Promise (<anonymous>)
#29 128.7     at /keep-core/solidity/random-beacon/node_modules/proper-lockfile/lib/adapter.js:30:25
#29 128.7     at writeValidations (/keep-core/solidity/random-beacon/node_modules/@openzeppelin/hardhat-upgrades/src/utils/validations.ts:35:24)
#29 128.7     at async OverriddenTaskDefinition._action (/keep-core/solidity/random-beacon/node_modules/@openzeppelin/hardhat-upgrades/src/index.ts:82:5)
#29 128.7     at async Environment._runTaskDefinition (/keep-core/solidity/random-beacon/node_modules/hardhat/src/internal/core/runtime-environment.ts:219:14)
#29 128.7     at async Environment.run (/keep-core/solidity/random-beacon/node_modules/hardhat/src/internal/core/runtime-environment.ts:131:14)
#29 128.7     at async SimpleTaskDefinition.action (/keep-core/solidity/random-beacon/node_modules/hardhat/src/builtin-tasks/compile.ts:989:37)
#29 128.7     at async Environment._runTaskDefinition (/keep-core/solidity/random-beacon/node_modules/hardhat/src/internal/core/runtime-environment.ts:219:14) {
#29 128.7   code: 'ERELEASED'
#29 128.7 }
#29 128.9 error Command failed with exit code 1.
#29 128.9 info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Once I re-run, the build have passed.
5. For me, I was getting error 500 in the ethereum block explorer in details of every block except of block 0.
6. When I was trying to reproduce the issue from point 4, by executing docker-compose build --no-cache, I got a different issue, on an earlier stage:

=> [stage-1 11/45] WORKDIR /solidity-contracts                                                                       0.0s 
 => [stage-1 12/45] RUN git init                                                                                      0.2s 
 => [stage-1 13/45] RUN yarn install                                                                                103.4s 
 => [stage-1 14/45] RUN yarn build                                                                                   88.9s 
 => [stage-1 15/45] RUN yarn link                                                                                     0.6s 
 => ERROR [stage-1 16/45] RUN yarn prepack                                                                           18.2s 
------                                                                                                                     
 > [stage-1 16/45] RUN yarn prepack:                                                                                       
#22 0.507 yarn run v1.22.19                                                                                                
#22 0.676 $ tsc -p tsconfig.export.json && hardhat export-artifacts export/artifacts
#22 18.02 An unexpected error occurred:
#22 18.02 
#22 18.10 SyntaxError: Unexpected token g in JSON at position 1092
#22 18.10     at JSON.parse (<anonymous>)
#22 18.10     at readValidations (/solidity-contracts/node_modules/@openzeppelin/hardhat-upgrades/src/utils/validations.ts:34:23)
#22 18.10     at async OverriddenTaskDefinition._action (/solidity-contracts/node_modules/@openzeppelin/hardhat-upgrades/src/index.ts:49:5)
#22 18.10     at async Environment._runTaskDefinition (/solidity-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:219:14)
#22 18.10     at async Environment.run (/solidity-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:131:14)
#22 18.10     at async SimpleTaskDefinition.action (/solidity-contracts/node_modules/hardhat/src/builtin-tasks/compile.ts:1438:7)
#22 18.10     at async Environment._runTaskDefinition (/solidity-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:219:14)
#22 18.10     at async OverriddenTaskDefinition._action (/solidity-contracts/node_modules/hardhat-contract-sizer/tasks/compile.js:8:3)
#22 18.10     at async Environment._runTaskDefinition (/solidity-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:219:14)
#22 18.10     at async Environment.run (/solidity-contracts/node_modules/hardhat/src/internal/core/runtime-environment.ts:131:14)
#22 18.18 error Command failed with exit code 1.
#22 18.18 info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

I run docker-compose build --no-cache again and didn't get any errors this time.

/ethereum/.data/
/bitcoin/.data/
/docker/bitcoin/.data/
/docker/ethereum/.data/
Copy link
Contributor

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).

Copy link
Author

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.
@beaurancourt
Copy link
Author

Not necessarily in this PR, but we should remember about updating README.

💯

As I didn't work with submodules in a long time, I didn't remember initially to execute git submodule init and git submodule update in the root folder. We could add those commands to the Testing section in the PR description.

Added!

Error: Lock is already released

I see this too sometimes, totally unclear as to why; it goes away if i just rebuild

For me, I was getting error 500 in the ethereum block explorer in details of every block except of block 0.

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

SyntaxError: Unexpected token g in JSON at position 1092

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!
@beaurancourt
Copy link
Author

What do we plan to do about the failed CI jobs? Are they still relevant or should we remove them?

Nuking them: 8c71a0e

Copy link
Contributor

@michalinacienciala michalinacienciala left a 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/
Copy link
Contributor

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/
Copy link
Contributor

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/
Copy link
Contributor

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.

@michalinacienciala
Copy link
Contributor

BTW, @nkuba / @pdyraga, before merging this PR we'll need to turn off the branch protection rules related to the checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants