Rename fields in shared interfaces

cartesi / rollups

Cartesi Rollups

30 stars 12 forks source link

Rename fields in shared interfaces #10

Closed gligneul closed 1 year ago

gligneul commented 1 year ago

📚 Context

@diegonehab pointed out that keccakInHashesSiblings in OutputValidityProof is confusing because it doesn't represent what the variable actually is.

There is also some confusion regarding input indices being local or global.

These variables are present in the OutputValidityProof structure across the Cartesi Rollups infrastructure, so this PR involves multiple repositories.

✔️ Solution

We should rename keccakInHashesSiblings to outputHashInOutputHashesSiblings. We should rename the <Output/Input>Index to <Output/Input>IndexWithinEpoch or epoch<Output/Input>Index.

📈 Subtasks

[ ] Rename it in grpc-interfaces repo
[ ] Rename it in server-manager (machine-emulator repo)
[ ] Rename it in host-server-manager (host-server-manager repo)
[ ] Rename it in GraphQL API (offchain/graphql-server)
[ ] Rename it in PostgreSQL (offchain/data)
[ ] Rename it in Broker (offchain/rollups-events)
[ ] Rename it in onchain code

gligneul commented 1 year ago

I created a PR in the gRPC interfaces repository. I will update the other APIs once we reach a consensus and merge the PR.

guidanoli commented 1 year ago

Replicating here the comment I made in https://github.com/cartesi/grpc-interfaces/pull/13:

I think it makes sense to rename InputIndex to InputIndexWithinEpoch, to differentiate it from the index of the input in the input box (which we may also call "global").

For the OutputIndex, however, I think it wouldn't make sense to rename it to OutputIndexWithinEpoch, because it could lead to misunderstanding. One could think that outputs are indexed by a serial number that gets zeroed every new epoch. Rather, this is the index of the output in the array of outputs generated by the input at InputIndexWithinEpoch.

My suggestion is to rename OutputIndex to OutputIndexWithinInput.

gligneul commented 1 year ago

I merged the PR on https://github.com/cartesi/grpc-interfaces/pull/13. I will apply this modification to the rollups repository (offchain and onchain code). I also plan to migrate the host-server-manager to this repository, so all our changes will be in one place. This will be done in the following issue: https://github.com/cartesi/rollups/issues/23

gligneul commented 1 year ago

The offchain changes were merged in #122. The onchain changes will be tracked in #123.