Forge has the forge bind command, which generates a Rust crate with the bindings.

I think we should create a crate just with the bindings. To do that, I think we could either extract this build.rs logic into this "contracts bindings" crate, or use forge bind instead. Guilherme generated Rust bindings for rollups-contracts using forge bind.

Sure, forge bind seems much easier to do. I was maintaining this build.rs because Arena was responsible for deploying the contracts and thus needed the abis. It's no longer the case that Arena now isn't responsible for deploying the contracts.

I like the way you've changed the arena/player interfaces, and the way you more explicitly split the reading and sending functionalities. I'll propose one further change, let me know what you think.

My understanding is that the Arena object has a bunch of reading methods. These methods are called beforehand, to fetch the state. Then, we call react on Player, passing this state. Furthermore, Arena also implementes the ArenaSender trait, and the Player stores it internally.

One suggestion is to really split the Arena object in two. One for reading states, that doesn't really implement any trait. Something like StateReader, or ArenaReader, or some better name. Then there's the ArenaSender trait, which is implemented by a different object. I believe this split is already kinda there, I'm suggesting we really split them in different objects and files. I may be missing something that makes this impossible.

Also, there's another interesting design pattern in Rust which may be interesting to experiment. Currently, we have something like:

trait ArenaSender {}

struct Player<A: ArenaSender> {
    arena_sender: A
}

impl<A: ArenaSender> Player<A> {
    fn new(arena_sender: A) -> Self {
        Self { arena_sender }
    }

    fn react() {}
}

Instead, we can do:

trait ArenaSender {}

struct Player {}

impl Player {
    fn new() -> Self {
        Self {}
    }

    fn react(arena_sender: impl ArenaSender) {}

    // or
    fn react<A: ArenaSender>(arena_sender: A) {}
}

I'll certainly split the reader and the sender in different files and objects. I'm curious about what advantage you think we can benefit from the second design pattern? Is it that the Player object is no longer bundled to a specific ArenaSender implementation? So the Player can freely react using different ArenaSender implementation (possible mocking version for testing purpose) at runtime?

For a later PR, we could consider generating this file automatically from some configuration, but I'm not sure.

It would be nice have different constants for testing. Let's think more on this later!

I think this method could be an implementation of std::default::Default.

I think we can avoid Box<dyn Error>, by either using anyhow crate (easiest, and it's probably ok), or by generating specific error types with thiserr.

We should probably change this from_block(0) to the latest block we've queried. I suggest the following. Since there are many calls, and the only public method is pub async fn fetch_from_root:

• creates a new field in struct StateReader called last_queried_block.
• in fetch_from_root, query the current latest block, and save it in a local variable latest_block.
• pass this queried latest_block to all nested functions, since the latest block may change while we're querying along the chain of function calls. Change all the queries to_block to use latest_block.
• change all requests to query from to use self.last_queried_block.
• after finishing querying the state fetch_from_root updates self.last_queried_block to latest_block before returning.

We might have to change &self to &mut self, or use a Mutex. Both makes a lot of sense, since with this change fetch_from_root calls have to be mutually exclusive. With mutex, we'd need to acquire the lock before we do anything, and only release it once we finish everything. The &mut self approach accomplishes the same thing at the type system level (but now we won't be able to share pointers to StateReader ). I think I prefer the &mut self approach.

If you want to get rid of Box<dyn Error>, we can create an associated type:

trait T {
    type Error;

    async fn foo(
         &self,
    ) -> Result<(), Self::Error>;
}

struct S {}

impl T for S {
    type Error = MyConcreteError;
    // ...
}