txpipe / dolos

Cardano Data Node
https://dolos.txpipe.io
Apache License 2.0
65 stars 18 forks source link

utxorpc sync returns intersected point first #294

Open Quantumplation opened 2 months ago

Quantumplation commented 2 months ago

The cardano node chainsync protocol uses the convention that the intersected point is agreed by both parties to be in the past; and so it doesn't make sense to send that block again, and the next block is the one after the intersected point.

However, it appears the dolos utxo RPC api returns the intersected point as the first event.

After speaking with @scarmuega this seems to be unintentional, so we should fix this to conform with the chainsync miniprotocol convention.

There are multiple benefits to this convention:

Quantumplation commented 2 months ago

@scarmuega happy to fix this, but I had some questions about where specifically to fix it;

I could implement it just for this utxorpc call, here: https://github.com/txpipe/dolos/blob/main/src/serve/grpc/sync.rs#L164-L165

I could implement it here, by skipping over the first entry in the WalStream: https://github.com/txpipe/dolos/blob/main/src/wal/stream.rs#L17-L20

or I could implement it here, in the redb implementation of wal.crawl_from: https://github.com/txpipe/dolos/blob/27d91af7dd71ab76b49776fbaa58870e64dc4d7c/src/wal/redb.rs#L204

Which would you prefer, since you have a wider perspective over how these are used.

scarmuega commented 2 months ago

@Quantumplation thanks for reporting and the fix would be very welcome. I prefer if we apply the change within the serve module (your first suggestion) because the alternative could have implications all around the codebase which already depends on the current behavior.

Quantumplation commented 2 months ago

https://github.com/txpipe/dolos/pull/298