refactor: refactor build and tests pipeline by GCdePaula · Pull Request #94 · cartesi/dave

GCdePaula · 2024-12-22T00:48:41Z

chore(prt-tests): remove --platform from Dockerfile

GCdePaula · 2024-12-22T01:13:29Z

So far I've:

introduced just and rewrote many makefiles into justfiles;
created a single cargo workspace, and put all crates in repo under this workspace;
reorganized directories (minor);
built a development Dockerfile with nice caching and all dependencies (including the current branch of the Cartesi machine, which it builds), as well as building Dave.

One possible next step is removing the usage of docker in our make/just targets. Currently, we have a Dockerfile that installs all dependencies, builds the repo and run tests. Our make targets invokes docker (docker build then docker run).

I think a better workflow just invokes the intended commands, without docker, and assumes all dependencies are there (i.e. the target just test directly runs our test scripts; it assumes all the dependencies are there). Then we also provide a docker image which has these dependencies in case the developer doesn't have these dependencies installed in their system (docker run dave:dev just test).

GCdePaula · 2024-12-26T20:58:23Z

Implemented anvil deployments as a state dump and load, as explained here.
Changed time from timestamps to block number, and fixed tests (vm.warp -> vm.roll). I think this is the most critical part of the PR. We'd planned on changing this; I didn't intend to change it now, but thew previous item required it.
Rewrote the way we build Cartesi "programs". So far, only echo is using this new pipeline (check test/programs).
Made node state directory configurable.
Running integration tests outside docker on my machine:

just bind && just -f test/programs/justfile build-echo && just -f prt/tests/rollups/justfile test-echo

GCdePaula · 2025-01-07T13:38:06Z

This PR is getting close to completion.

Feature-wise, the main change is from block timestamp -> block number, which is a delicate change. The rest is mainly refactoring and changes to how we invoke programs.

Running tests on the host:

$ just setup
$ just build && just -f prt/tests/rollups/justfile test-echo

Running tests in docker:

$ just run-dockered just -f prt/tests/rollups/justfile test-echo

stephenctw · 2025-01-08T08:47:52Z

Cargo.toml

+
+# rollups-node
+rollups-blockchain-reader = { version = "0.1", path = "cartesi-rollups/node/blockchain-reader" }
+rollups-compute-runner = { version = "0.1", path = "cartesi-rollups/node/compute-runner" }


Now I think that compute-runner should be named to something like prt-runner.

stephenctw · 2025-01-08T08:57:24Z

cartesi-rollups/node/machine-runner/src/lib.rs

 use rollups_state_manager::{InputId, StateManager};

-// gap of each leaf in the commitment tree, should use the same value as CanonicalConstants.sol:log2step(0)
+// gap of each leaf in the commitment tree, should use the same value as CanonicalConstants.sol:log2step(0)a


was the last a a typo or was it put there on purpose?

GCdePaula · 2025-01-16T10:26:02Z

With newer foundry version, we started hitting smart contracts size limit. The solution (great catch @stephenctw!) is to add the option --disable-code-size-limit in anvil, both when storing and loading.

An important consideration is whether this limit will appear when deploying to main net (or whether it's only our tests that hit this limit).

* introduce `just` and rewrite makefiles into justfiles * unify all cargo workspaces into a single cargo workspace; * unify all Dockerfiles into a single Dockerfile; * implement anvil deployments as state dumps and loads; * update `blockchain-reader` tests to load anvil state; * Change PRT time from timestamp to block number; * refactor `echo` program build; * implement node state directory configuration; * update PRT tests to newest changes; * simplify `machine-runner` test; * fix `persistent-state-access` tests; * add ci (check/fmt/build/test) with cache; * add machine bindings to workspace; * rename `rollups-compute-runner` to `rollups-prt-runner`; * extract `prt/tests` shared modules into `prt/tests/common` dir; * remove `step` dependency on `CanonicalConstants`; * remove unused files.

GCdePaula · 2025-01-16T11:58:03Z

This ended up being a larger PR than I'd imagined. It's mostly refactoring, except from the change of timestamp -> blocknumber, which is quite major, but the smart contracts were well implemented enough that it was a simple change (but the clients required more effort).

guidanoli · 2025-01-16T13:43:53Z

cartesi-rollups/contracts/justfile

+        --broadcast \
+        --sig 'run(bytes32,address)' \
+        {{INITIAL_HASH}} \
+        $(jq -r '.transactions[] | select(.transactionType=="CREATE").contractAddress' broadcast/InputBox.s.sol/{{ANVIL_CHAIN_ID}}/run-latest.json) \


To avoid this, we could join these two Solidity scripts into one.

I think I purposely want them to be two separate blocks so that the the first epoch already have inputs to process

Ok! I've kept as @stephenctw prefer, and we can keep discussing this later. I'm fine either way.

stephenctw · 2025-01-17T13:11:32Z

This ended up being a larger PR than I'd imagined. It's mostly refactoring, except from the change of timestamp -> blocknumber, which is quite major, but the smart contracts were well implemented enough that it was a simple change (but the clients required more effort).

I think there's one more place to update timestamp -> blocknumber
https://github.com/cartesi/dave/blob/feature/cleanup/prt/client-lua/player/reader.lua#L81

stephenctw · 2025-01-17T13:03:59Z

justfile

+    just -f ./prt/contracts/justfile clean-bindings
+bind-prt:
+    just -f ./prt/contracts/justfile bind
+


Should we also add the tests related just targets? Something like

test-prt-rollups-echo: just -f prt/tests/rollups/justfile test-echo

?

I think we can add it in a later PR!

GCdePaula force-pushed the feature/cleanup branch from 5bcc9fb to d92a44c Compare December 22, 2024 01:14

GCdePaula changed the title ~~(chore): refactor build pipeline~~ chore: refactor build pipeline Dec 22, 2024

GCdePaula force-pushed the feature/cleanup branch 3 times, most recently from d21df14 to 85eb123 Compare December 26, 2024 20:58

GCdePaula force-pushed the feature/cleanup branch 2 times, most recently from 83f676e to 114708d Compare January 7, 2025 13:34

GCdePaula requested review from guidanoli and stephenctw and removed request for stephenctw January 7, 2025 13:38

stephenctw self-assigned this Jan 13, 2025

stephenctw linked an issue Jan 13, 2025 that may be closed by this pull request

Create Rust workspace at the project root, and a Makefile. #79

Closed

GCdePaula marked this pull request as ready for review January 13, 2025 12:25

stephenctw reviewed Jan 13, 2025

View reviewed changes

GCdePaula force-pushed the feature/cleanup branch 4 times, most recently from e9bd3b7 to fa2efd0 Compare January 15, 2025 10:34

GCdePaula force-pushed the feature/cleanup branch 3 times, most recently from eff31e4 to ab86aa2 Compare January 16, 2025 11:28

GCdePaula force-pushed the feature/cleanup branch from ab86aa2 to 45f3409 Compare January 16, 2025 11:55

GCdePaula changed the title ~~chore: refactor build pipeline~~ refactor: refactor build and tests pipeline Jan 16, 2025

guidanoli reviewed Jan 16, 2025

View reviewed changes

stephenctw approved these changes Jan 17, 2025

View reviewed changes

GCdePaula merged commit 513ebc0 into main Jan 17, 2025
1 check passed

GCdePaula deleted the feature/cleanup branch January 17, 2025 14:37

Conversation

GCdePaula commented Dec 22, 2024

Uh oh!

GCdePaula commented Dec 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GCdePaula commented Dec 26, 2024

Uh oh!

GCdePaula commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephenctw Jan 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GCdePaula commented Jan 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GCdePaula commented Jan 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenctw commented Jan 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GCdePaula commented Dec 22, 2024 •

edited

Loading

GCdePaula commented Jan 7, 2025 •

edited

Loading

stephenctw Jan 8, 2025 •

edited

Loading

GCdePaula commented Jan 16, 2025 •

edited

Loading