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

Make our tools less smart #447

Open
0xaatif opened this issue Jul 25, 2024 · 2 comments
Open

Make our tools less smart #447

0xaatif opened this issue Jul 25, 2024 · 2 comments

Comments

@0xaatif
Copy link
Contributor

0xaatif commented Jul 25, 2024

In #375 (comment), I make the broad suggestion to decompose our "cleverer" functionality into simpler commands, e.g splitting:

  • Circuit generation, caching, loading
  • Making rpc calls, assembling a witness (maybe)

In #413 I regressed functionality that we don't test in CI - talking to RPC nodes and assembling an output.
This is hard to test in CI because:

  • RPC nodes are precious - we don't like to expose them, and using them in CI means we have to be more careful about that.
  • We can't/don't want to spin up an RPC node in CI.
  • We don't want to mock a whole RPC node.

However, because this part of trace_decoder/zero_bin is all bundled together, we can't catch regressions without solving some of the above.

We can break out our Public API, and get benefits in:

  • a simpler codebase
  • a more testable codebase
  • better user/beginner knowledge of what is going on behind the scenes

at relatively low UX cost.

@atanmarko
Copy link
Contributor

atanmarko commented Jul 25, 2024

RPC nodes are precious - we don't like to expose them, and using them in CI means we have to be more careful about that.
We can't/don't want to spin up an RPC node in CI.
We don't want to mock a whole RPC node.

Why not? We have already done that with the real node here, and here is the regression test that was missing for your usecase.

That aside, I agree that we should discuss functionality split.

EDIT: we now use kurtosis, before we used docker-compose to spin up test network for real regression test.

@atanmarko
Copy link
Contributor

Making rpc calls, assembling a witness (maybe)

This is what rpc binary is doing at the moment.

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

No branches or pull requests

3 participants