getEventsAtBlockHeightRange does not indicate if some blocks were past the latest block height

[chriswayoub]

Problem

When calling getEventsAtBlockHeightRange, if the latest block height is 1000 and you request 999 - 1010, the call returns successfully and you have no indication that blocks 1001 - 1010 were not actually available. This is an issue when you are continuously querying the latest block(s) for events. On certain requests, the latest blocks may not be available (I'm assuming this depends on which access node you happen to to hit). You can't be sure if the blocks were not available yet, or if there were no events in those blocks.

The Go SDK handles this by returning an array of every block that was queried along with a nested array of the events for that block. If you request something past the latest block height, the blocks that are not available will not be included in the results. So in the example above, you would only receive results for blocks 999 - 1000, and 1001-1010 would be missing from the results (which is what I would expect).

I'm not sure what the solution for this is with the current JS getEventsAtBlockHeightRange if you want to maintain backward compatibility, since it is just an array of all events (they are not grouped in the same way that the Go SDK results are).

Steps to Reproduce

const latestBlock = await sdk.send(
      await sdk.build([
        sdk.getBlock(true),
      ]), {
        node: this.accessNode,
      }).then(sdk.decode)

const events = sdk.getEventsAtBlockHeightRange('testEvent', latestBlock.height, latestBlock.height + 10),

// at this point, events will contain any events of 'testEvent' type for the latest block
// there is no indication that the other 9 blocks that were queried were not available.

I think this may be related to issue #583

[MaxStalker]

@chriswayoub why would you go 10 blocks above latest block if it's the upper limit? More logical would be to call sdk.getEventsAtBlockHeightRange('testEvent', latestBlock.height-10, latestBlock.height)

[chriswayoub]

I don't know the internals of how the access nodes operate, but I have received a different latestBlock across multiple requests. It seems like some access nodes could potentially be behind others (I'm assuming there is a cluster of access nodes that are being load balanced). So even when you're only reading events up to the latest block height, if you happen to hit an access node that is lagging behind the one that you queried for the latest block height, then you will get no results for getEventsAtBlockHeightRange. You have no way to know if this is because the access node did not have the requested blocks yet, or if there were really no events for the requested blocks.

[bluesign]

I always use something like: start = max(last block that had event , sealed_block_height-200) end = start + 249

you just need to get sealed_block_height once in couple of event query.

[chriswayoub]

I agree that there are workarounds to handle this scenario in the calling code, but I still think it's an issue in the underlying response that is returned when querying for events. I didn't dig into the raw response yet, but I'm assuming that the problem is in the constructResponse method that reduces the returned blocks down to a flat list of events (rather than each block with an array of events contained inside of it). The Go SDK returns a result for each valid queried block, and I would assume that the raw response (before constructResponse transforms it) does the same thing.

The main headache is that I'm not sure how to fix this without changing the structure of the response returned by getEventsAtBlockHeightRange. Maybe to maintain backward compatibility, some additional properties could be added to the response to indicate the range of blocks that were actually returned. So in addition to events there could be something like startHeight and endHeight included on the response object. Personally, I think that having an array of the returned blocks with a nested array for the events in that block (or an empty array if there were no events) is a cleaner solution, but I'm also not a fan of breaking changes without a good reason...