onflow / flow-go

A fast, secure, and developer-friendly blockchain built to support the next generation of games, apps, and the digital assets that power them.
GNU Affero General Public License v3.0
530 stars 176 forks source link

[Access] Change SendAndSubscribe endpoint's MessageIndex to start at 0 #6575

Open peterargue opened 1 week ago

peterargue commented 1 week ago

Problem Definition

The SendAndSubscribeTransactionStatuses endpoints includes a MessageIndex field to help the client check that they received all messages. Currently, this value starts at 1 instead of 0, which is a little confusing since almost all iteration fields start at 0, and it's documented that it starts at 0. https://github.com/onflow/flow/blob/0b0946dbb2683cec184522b0e3e88246b076a2dc/protobuf/flow/access/access.proto#L753-L754

Other endpoints are documented to start at 1, but actually start at 0: https://github.com/onflow/flow/blob/0b0946dbb2683cec184522b0e3e88246b076a2dc/protobuf/flow/executiondata/executiondata.proto#L469-L470 https://github.com/onflow/flow/blob/0b0946dbb2683cec184522b0e3e88246b076a2dc/protobuf/flow/executiondata/executiondata.proto#L621-L622

Proposed Solution

Let's update the logic and docs to use 0 as the first value for all endpoints.

This is the current logic for SendAndSubscribeTransactionStatuses https://github.com/onflow/flow-go/blob/0dde73002b873c30a94e995f80b97ad2cc42355e/access/handler.go#L1435-L1450

We could update it to be similar to the logic in other endpoints https://github.com/onflow/flow-go/blob/f6814f803f5a0c48190e0eb68eb220e602577707/engine/access/state_stream/backend/handler.go#L417-L427

illia-malachyn commented 3 days ago

If you run new examples https://github.com/onflow/flow-go-sdk/pull/794 via make stream-account-statuses, you will run into an error msg received out of order. The current implementation expects it to start from 0, but it actually starts from 1 even though the tests expect it to start from zero. So, this is the bug and it has to be fixed asap