Using Celo and Ethereum at the same time

[blackghost1987]

Is your feature request related to a problem? Please describe.
I've tried to improve an application that can connect to the Ethereum network, to be able to connect to the Celo network as well, at the same time. For this I had to add the celo feature to my dependencies. Now the Ethereum connection (which was working fine before) is failing with the following ethers::providers::ProviderError error:

JsonRpcClientError(SerdeJson { err: Error("missing field `randomness`", line: 1, column: 1563), text: "{\"difficulty\":\"0x2\",......

As I found out "randomness" is a Celo specific field in the Block, so it's normal that it will be missing from an Ethereum block.

Describe the solution you'd like
I'd like to use Celo and Ethereum together, at the same time, in the same binary. I think the Celo specific fields should be optional, or there should be a different type for Celo blocks, by using something like Either or a generic solutions. These are just my rough ideas, I'm not well-versed enough in library development to determine the best solution for this, I hope the community can come up with something easy to use and maintain.

Describe alternatives you've considered
I tried to import the library twice, with different feature-sets, but unfortunately Rust doesn't support that. I tried to look it up and it seems to be a deliberate choice, because features should be "additive" in concept. I think this means that enabling a feature should not break an already working code. A feature could be enabled by any dependency downstream, even without the main program depending on it, so the "celo" feature being used should not break Ethereum support.

Additional context
If this issue takes a long time to fix it, or if it will be considered "unfixable" (which I hope is not the case), then at the minimum this would require a better error message, the sooner the better. Debugging the cryptic message missing field could be an issue for newcommers, if they accidentally mess up the initial configuration of the features.

[mattsse]

yeh baking in mandatory fields via celo cfg will break if you need to use that for different networks.

this also came up recently where L2s also include their specific fields, so after a brief discussion we came to the conclusion that we should catch all other fields in an additional field via serde(other)

[blackghost1987]

Thanks for the quick work, I tested it and this is actually not solving my problem. It's a nice idea to put Celo specific fields in an "other" field, but only doing this when the Celo feature is NOT enabled is actually the opposite of what I need.

The change is useful, it actually makes it possible to communicate with the Celo network when the Celo feature is NOT enabled, because the randomness field will go into other. But it's still not possible to communicate with the Ethereum network when the Celo feature is enabled! In that case there's no other field, and the code always expects the randomness value, which is not there.

A solution to my problem would be always using the other field, regardless of what features are enabled, while also removing the randomness field (and other Celo specific fields) from the code. This way randomness will be an unknown field and go into other when present, but won't cause any issues when not present.

I know this would be a breaking change, because some existing code might rely on the randomness field being there, and after this change they would have to get it from the OtherFields mapping. But I can see no other solution for handling Ethereum and Celo with the same binary. What do you think?

[mattsse]

But it's still not possible to communicate with the Ethereum network when the Celo feature is enabled!

that's correct because the fields are mandatory

A solution to my problem would be always using the other field, regardless of what features are enabled

that's how it is intended.

I'd recommend to make another type that bundles all celo fields and use this

https://github.com/mattsse/ethers-rs/blob/8b1e73077a3a6a3452c46fe40b5b0b33945580c4/ethers-core/src/types/other.rs#L106-L125

you can even add something like

trait CeloExt {
   fn randomness(&self) -> Option<Randomness>;
}

and implement this on Block/ Transaction

imo that'd be useful directly in ethers-core