A simple mockup of L2 logic without proofs or signatures.

[Avi-D-coder]

Follow up tasks:

• Verify signatures where I have TODOs.
• Use https://github.com/rkyv/rkyv for inner messages that appear in a ZK context.
• Decide on a deposits model.
• Test the model.
• Maybe: Add a mock L1 server.
• Move to thiserror so we can consume typed JSON errors in the CLI.
• Try using https://github.com/paritytech/trie/tree/master/trie-db, is it suited for both server, and zk, neither, both?

[marijanp]

Obviously some things are missing like a token-id for all the transactions, but we didn't iron them out yet. Code is clear and easy to follow, important edge-cases have proper handling or have an according todo,.

[marijanp]

What if we introduced

pub enum Transaction {
   Deposit { .... },
   Transfer { .... },
   Withdraw { ... }
}

Then we could have a batch HashSet<Transaction>

[marijanp]

Oh I just saw transfers are in a HashSet, and the others in a vector.

[Avi-D-coder]

My thinking here is Deposit and Withdraw need separate handling.
Although perhaps Withdraw should also be a HashSet to check for duplication.

[marijanp]

Sure, but we could just match and dispatch accordingly. But let's move that to a later point, seems premature. Also, I think adding a timestamp to each of those types would make sense and put all of them in a hashset actually

[marijanp]

As a next step I would also move most code out of the main.rs and put it into a run function which expects parsed CLI-Args in a server module.

[asladeofgreen]

The types that flow in/out of server to/from an sdk could be factored out to a dedicated crate ... I began this process in my paradox implementation.

https://github.com/cspr-rad/paradox-4/tree/main/node/types/src/external

[asladeofgreen]

General comment in respect of the handlers. Typically they should be a thin layer, the business logic should be handled by an inner layer that can be tested outside of the network context. But it's early days.

[asladeofgreen]

Another general comment in respect of the handlers. The input/output request/response types could be wrapped in generic ServerRequest & ServerResponse types. These types would provide a contextual envelope.

For an example of what I mean, check out the core external types exposed in my paradox implementation.

[Avi-D-coder]

Another general comment in respect of the handlers. The input/output request/response types could be wrapped in generic ServerRequest & ServerResponse types. These types would provide a contextual envelope.

I'd suggest using a middle layer there.

https://docs.rs/axum/latest/axum/middleware/index.html

[asladeofgreen]

Finished my review. Most of my comments relate to code structure and are nit-picky ... but there again I am very nit-picky ! Also most of my comments should be taken with a pinch of salt as I know that you guys are just getting things up and running.

I would ask you to be cognizant of the fact that a successful L1/L2 will result in the development of several SDKs, e.g. rust, typescript, python ... etc. Therefore it is useful to structure the code in such a way that makes it as easy as possible for an SDK to be built. This typically means factoring out the over the wire types to a dedicated crate ... sometimes even with some meta-programming tricks so as to forward engineer the over the wire typeset in other programming language bindings. Also on this note, the cli program should be as thin as possible and depend upon the sdk ... you quite often see cli/sdk entanglements that make it difficult to use the sdk in standalone mode (e.g. from a trading bot).

[Avi-D-coder]

L1/L2 will result in the development of several SDKs, e.g. rust, typescript, python

My thinking here is we are going to call into a native Rust or WASM lib for sigs anyway, those SDKs can be thin wrappers around a Rust lib. If we try and make over the wire types multi lang compatible we can not use things like rkyv or sum types. I'd have the Rust lib handle requests for this reason.

[Avi-D-coder]

I switched to typed routes, and added logging and two tests.

[marijanp]

Sure, but we could just match and dispatch accordingly. But let's move that to a later point, seems premature. Also, I think adding a timestamp to each of those types would make sense and put all of them in a hashset actually

[asladeofgreen]

Nice. Will be running tests and going deeper into business logic but am approving.

[Avi-D-coder]

So I am thinking we currently don't care about CI failing.

[marijanp]

can you

nix flake check
nix build .#kairos

if that builds, you are good to merge I would say.