feat(tools): substrate test ledger

[RafaelAPB]

Substrate-based blockchains like Polkadot are becoming increasingly relevant. However, Cactus does not have tools to support developing Substrate-based connectors, such as a test ledger.

This draft PR provides a first attempt at defining a Substrate test ledger, that allows to programmatically instantiate the Substrate node template (https://github.com/substrate-developer-hub/substrate-node-template).
Thus, it contains a Dockerfile with the test ledger and the test-ledger file that contains the execution logic.

I would appreciate feedback on the tools/docker/substrate-all-in-one/Dockerfile and packages/cactus-test-tooling/src/main/typescript/substrate-test-ledger/substrate-test-ledger.ts files prior to merging.

Notes:
At the moment a bug occurs in the Dockerfile at RUN rustup update nightly

info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2021-08-26, rust version 1.56.0-nightly (0afc20860 2021-08-25)
info: downloading component 'rust-std' for 'wasm32-unknown-unknown'
info: downloading component 'cargo'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: removing previous version of component 'rust-std' for 'wasm32-unknown-unknown'
info: rolling back changes
error: could not rename component file from '/usr/local/rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/wasm32-unknown-unknown/lib' to '/usr/local/rustup/tmp/jxirtjwr7gf70a8r_dir/bk'
error: caused by: other os error
error: backtrace:
error: 0: error_chain::backtrace::imp::InternalBacktrace::new
1: rustup::utils::utils::rename
2: rustup::dist::component::transaction::Transaction::remove_dir
3: rustup::dist::component::components::Component::uninstall
4: rustup::dist::manifestation::Manifestation::update
5: rustup::dist::dist::update_from_dist_
6: rustup::install::InstallMethod::install
7: rustup::toolchain::DistributableToolchain::install_from_dist
8: rustup::cli::rustup_mode::update
9: rustup::cli::rustup_mode::main
10: rustup_init::main
11: std::rt::lang_start_internal::{{closure}}::{{closure}}
at rustc/c7087fe00d2ba919df1d813c040a5d47e43b0fe7/src/libstd/rt.rs:52
std::sys_common::backtrace::__rust_begin_short_backtrace
at rustc/c7087fe00d2ba919df1d813c040a5d47e43b0fe7/src/libstd/sys_common/backtrace.rs:130
12: main
13: __libc_start_main
14:

[RafaelAPB]

cc @petermetz @CatarinaPedreira

[petermetz]

@RafaelAPB I can chime in!
Fair warning: I'm a Polkadot/Substrate noob.

With the disclaimer above out of the way, here's the first (basic) question of mine:
Is this meant to mimic a ledger similar to the ones that a person would end up with if they went through one of these tutorials?

This?
https://substrate.dev/docs/en/tutorials/create-your-first-substrate-chain/
Or maybe this?
https://substrate.dev/docs/en/tutorials/start-a-private-network/

[RafaelAPB]

@petermetz yes, the "Create Your First Substrate Chain" one!

[petermetz]

@RafaelAPB Took a peek just now I think the issue might be that you didn't set the image on GHCR to public. By default it has a visibility setting of "private" so you need to update that.
I pushed a couple fixes to your branch to make it compile.
When I run the test it goes out with server error - Head "https://ghcr.io/v2/rafaelapb/cactus-substrate-aio/manifests/2021-08-26-first": denied which is what makes me think that it's a container repo visibility issue. Please double check the visibility and let me know and then we can take it from there.

[RafaelAPB]

@petermetz the image has not been submitted, because the build process fails. I'm trying to fix it (meanwhile @CatarinaPedreira might take this task).

[petermetz]

@petermetz the image has not been submitted, because the build process fails. I'm trying to fix it (meanwhile @CatarinaPedreira might take this task).

@RafaelAPB Oh, okay, now I get the full picture! I'll take a peek at the image build, maybe I can help.

[CatarinaPedreira]

@RafaelAPB @petermetz Yes, I will try to take a look at it in a bit and will report later :)

[petermetz]

@RafaelAPB @CatarinaPedreira FYI: You only need the wasm-pack tool if you plan on packaging some Rust code into Javascript WASM modules to be executed by NodeJS. If this is not the case then you could just remove that part from the image definition (Dockerfile)

[CatarinaPedreira]

@petermetz @RafaelAPB Sorry for taking so long, I only got to see this issue with full attention today.
I did some research on the error and found this similar issue rust-lang/rustup#2542.

It seems that the problem lies in the rename() function, more specifically in the fact that docker's filesystem behavior and rustup filesystem's behavior are not compatible.
There is no good solution yet, but the most suggested solution in this and other similar issues is uninstalling and reinstalling rustup.
But since this image is created from paritytech/ci-linux's image (https://github.com/paritytech/scripts/blob/master/dockerfiles/contracts-ci-linux/Dockerfile), which first installs nightly, how would I do this?
Having instructions to uninstall it and reinstall rustup it in our Dockerfile seems redundant, but I'm not sure what to do.

I will research a bit more and if I don't find any better solution I think I will do that. In the meantime, if you have any ideas, please say something :)

[RafaelAPB]

@petermetz thanks! Working on an issue with you having our back makes it much less daunting :)

@CatarinaPedreira thanks for taking this task :) Perhaps using another image? Peter's image on the Rust container could provide a good starting point, but I don't know if it has all the dependencies for the test ledger.

In the meantime it might be a good idea to open an issue at the repo responsible for the image.

[CatarinaPedreira]

@RafaelAPB no problem!
@petermetz In the meantime, I opened an issue at paritytech/scripts (paritytech/scripts#339), with no reply yet.
I found another possible parent image in docker hub, that is more specific for using and testing smart contracts (https://hub.docker.com/r/paritytech/contracts-ci-linux)!. I'm going to test and see if this one works for our image.

EDIT: I tested with the new parent image and the same error was provided (this time with a little more information - Invalid Cross-device link). I did some more research and it is an unsolved bug at kernel-level (the "redirect_dir" feature is disabled which causes problems in overlayfs).

So the suggestion was, again, uninstalling and reinstalling rustup in the dockerfile as a workaround.
However, as the new parent image (paritytech/contracts-ci-linux) always installs the latest nightly version, I don't think we need to run the "RUN rustup update nightly" instruction anyway.

I'm a bit of a docker noob, but from my understanding, every time we build our docker image, docker builds the parent image as well, right? So I think this is not a problem. We can just remove that specific instruction since nightly will always be updated due to the parent image. Please let me know if I'm wrong.

There is another bug in "RUN cargo build --release", but I'm going to tackle it right now.

[CatarinaPedreira]

I'm having a bit of difficulty with this new bug:

=> ERROR [ 8/10] RUN cargo build --release 1.9s

[ 8/10] RUN cargo build --release:
#11 1.196 error: could not find Cargo.toml in / or any parent directory

The image is now based in https://github.com/paritytech/scripts/blob/master/dockerfiles/contracts-ci-linux/Dockerfile, and I did not find any reference to a Cargo.toml file here.

Anyhow, here are some things I tried:

• Created a Cargo.toml file in substrate-all-in-one directory, and used the COPY instruction to copy it to the container.
• Added a "RUN cargo new node-template" instruction to the Dockerfile so that it creates a Cargo.toml file in the container

That didn't work. It seems that docker is not being able to find the Cargo.toml.

Also, @RafaelAPB, your docker-compose.yml is based in this one, right? (https://github.com/substrate-developer-hub/substrate-node-template/blob/master/docker-compose.yml).
It seems that they only use the docker compose file and no dockerfile.

On that note, I also tried changing the Cargo.toml file in substrate-all-in-one directory to be equal to the substrate one (https://github.com/substrate-developer-hub/substrate-node-template/blob/master/Cargo.toml), executed "docker-compose up" and it found the Cargo.toml, but provided this error:

Attaching to node-template-cactus
node-template-cactus | error: failed to load manifest for workspace member /var/www/node-template/node
node-template-cactus |
node-template-cactus | Caused by:
node-template-cactus | failed to read /var/www/node-template/node/Cargo.toml
node-template-cactus |
node-template-cactus | Caused by:
node-template-cactus | No such file or directory (os error 2)
node-template-cactus exited with code 101

Which makes sense since the members are not created there. But I'm not sure if it makes sense to create them manually?

Also, @RafaelAPB, two doubts:

• In line 6 of the docker-compose.yml, shouldn't it be the image created by our Dockerfile instead of the current one?
• The instruction "ENV CACTUS_CFG_PATH=/etc/hyperledger/cactus" is repeated in the Dockerfile. Is this a mistake?

Thanks, and sorry If I'm not making a lot of sense right now - I'm going to pause a little on this and be back in a few hours :)

[petermetz]

I'm a bit of a docker noob, but from my understanding, every time we build our docker image, docker builds the parent image as well, right? So I think this is not a problem. We can just remove that specific instruction since nightly will always be updated due to the parent image. Please let me know if I'm wrong.

@CatarinaPedreira I think the parent image doesn't get re-built, just pulled as is. E.g., the state it is in is determined at the time when whoever built that parent image did so. With that said if you are referencing a moving tag like latest for the version of the parent image then it might happen that the image behind the latest tag gets overwritten with a different build over time kind of resulting in the behavior that you described as it always being rebuilt.

Thanks, and sorry If I'm not making a lot of sense right now - I'm going to pause a little on this and be back in a few hours :)

No worries at all, it's a difficult topic from what I can tell so far! What does help with helping/debugging is if you also push the changes you made to the dockerfile so that I can try and reproduce the issues locally so I encourage you to do that every time you post a comment with an error/question/etc. :-)

[CatarinaPedreira]

@petermetz Ohh, ok, that makes sense. Thank you! :)
Yes, tomorrow I'll push the changes to this PR.

[CatarinaPedreira]

@petermetz Added a commit with changes mainly to the dockerfile and docker-compose.yaml.

Main changes:

• I removed instructions that were redundant from the Dockerfile, such as cargo build --release and ./target/release/node-template --dev --ws-external and left them only on the docker-compose.yml (these instructions were both in the Dockerfile and in the "command" instruction of the docker-compose.yaml (wich overrides the default command in Dockerfile));
• Changed the image used by docker-compose.yaml to be the one generated by our dockerfile (hyperledger/substrate-all-in-one:latest);
• Added a cargo.toml file

The dockerfile build works ok now (removed the rustup update instruction given that the parent image always strives to contain the latest nightly version as said in https://github.com/paritytech/scripts/tree/master/dockerfiles/contracts-ci-linux).

But this new cargo.toml file (same as https://github.com/substrate-developer-hub/substrate-node-template/blob/master/Cargo.toml) is the one causing errors right now ("no such file or directory" error in my last comment), when trying to build the container (docker-compose -f tools/docker/substrate-all-in-one/docker-compose.yaml up).

It makes sense to cause this error given that the members of the workspace, described in the Cargo.toml file, are not created in our substrate-all-in-one folder (as they are created in https://github.com/substrate-developer-hub/substrate-node-template).
I'm not sure what our Cargo.toml file should look like here, perhaps I have to create those directories locally and copy them to the container?

@RafaelAPB Did you create a cargo.toml but didn't commit it here?

[CatarinaPedreira]

A few days ago Polkadot announced https://github.com/paritytech/substrate-contracts-node which is essentially a minimal working node (based on the node template we were using) already with smart contract support, which they recommend for smart contract development and testing and thus is better for this case.
I followed the install instructions into the Dockerfile and the substrate contracts node is correctly installed (the image builds with no errors).

I still have a bug when running the image with docker compose, but I looked into it and apparently, the bug only pops up in some host operating systems. It seems that there is no permanent fix as well. Can you try to build the dockerfile and then run it with the docker-compose.yaml in your devices to see if you have an error there?

Thank you :)

[CatarinaPedreira]

@petermetz @RafaelAPB The container now works nicely if we run the container directly through the image (using "docker run -t -p 9944:9944 --name substrate-contracts-node hyperledger/substrate-all-in-one:latest" to run it other than using the docker compose file).

This is a temporary fix so that I can move forward with the work, but when I have time I can come back to fix the YAML file to make the substrate-all-in-one folder more coherent with the others.

[RafaelAPB]

@petermetz this PR is ready for review. Thanks for all the brilliant contributions, @CatarinaPedreira

[codecov-commenter]

Codecov Report

Merging #1274 (a96055b) into main (2e793b9) will decrease coverage by 0.02%.
The diff coverage is 63.21%.

@@            Coverage Diff             @@
##             main    #1274      +/-   ##
==========================================
- Coverage   70.42%   70.39%   -0.03%     
==========================================
  Files         301      302       +1     
  Lines       10815    10902      +87     
  Branches     1320     1332      +12     
==========================================
+ Hits         7616     7674      +58     
- Misses       2505     2526      +21     
- Partials      694      702       +8     

Impacted Files	Coverage Δ
...ipt/substrate-test-ledger/substrate-test-ledger.ts	62.35% <62.35%> (ø)
...tus-test-tooling/src/main/typescript/public-api.ts	100.00% <100.00%> (ø)
...t-tooling/src/main/typescript/common/containers.ts	60.07% <0.00%> (+0.39%)	⬆️
...main/typescript/rustc-container/rustc-container.ts	68.53% <0.00%> (+2.24%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e793b9...a96055b. Read the comment docs.

[CatarinaPedreira]

@RafaelAPB @petermetz I applied the requested changes (except that TO-DO which is out of scope for this PR) and updated cactus-test-tooling's public-api. Hopefully the build will pass now 🤞
Can you approve the workflow run again Peter?

[CatarinaPedreira]

@petermetz @RafaelAPB My latest commits fix the errors I had on the latest build. I'm hopeful everything will work now!
Can you approve the workflow run again Peter?

[petermetz]

@CatarinaPedreira Please also make sure to fix the DCO check!

[CatarinaPedreira]

@petermetz Done :) Is this ok to merge? I believe the builds were passing

[petermetz]

@CatarinaPedreira Cool, thank you! Just have one nit-pick left (see comments inline) once you address that please:

1. squash the changes into a single commit
2. that one commit message created after teh squash needs to have the description that you typed into the PR description already (but it's completely missing in the git commit message)
3. Then we are good to merge

[CatarinaPedreira]

@petermetz @RafaelAPB All done :)

[petermetz]

@CatarinaPedreira Thank you very much, LGTM!

[CatarinaPedreira]

Hi @takeutak :) Can you review this PR?

[takeutak]

@CatarinaPedreira Thanks for contributing! LGTM.