When re-creating listeners in EVMConnect/EthConnect/FabConnect use the last event block

[peterbroadhurst]

See #1531

Every time Core starts, it confirms all the listeners exist that it needs for all of:

• Contract listeners registered by applications
• BatchPin listeners for any multi-party namespaces
• Token listeners - indirectly via /activatepool
  • Not changed in this PR - see Migration to v1.3.0 can cause missed events when contract listeners are listening from latest #1531 comments for justification

This is needed for two cases:

1. In case the listeners were deleted operationally for any reason (intentionally or otherwise) - such as a DR scenario
2. In the v1.2->v1.3 migration, where we intentionally orphan all the core-wide listeners in favor of namespace-specific ones

In both of these cases, the logic was naive for the newest/latest case - where it treats "now" as the latest event.

That is a problem, as it might be a long time since the original listener was created, and it might have delivered many events already. In which case there might be "gaps" we skip over between the last event detected by the old listener, and the first event we detect on the new one.

This PR closes that window, by looking at the protocolId of the last event - which is architecturally required to be a lexicographically sorted string identifying the position of the event on the blockchain.

If there's been a previous event, that's used by the blockchain connector as the fromBlock for the new listener.

Also, if "oldest" or an explicit block number is used, and the block number is before the block determined by the above process, then the newer block is used.

[EnriqueL8]

Why are we not using the multiparty listener ID here? We don't have access to it?

[peterbroadhurst]

Specifically the thing that distinguishes the blockchainevents for the multi-party BatchPin events, is the lack of a listener reference (there isn't one in the FF model for the BatchPin case).

[EnriqueL8]

Ah makes sense

[peterbroadhurst]

PR has been updated based on the conversation here, so that in the case a specific block number is supplied the recreated listener will still restart on the block before the last event (if that's after the specified block): #1531 (comment)

[EnriqueL8]

Will be testing this one today e2e

[EnriqueL8]

Re-reviewing I thought about a crazy edge case... The protocol ID is block number/transaction number /event number so if we recreate a listener with block number of the last protocol ID but we haven't consumed all the events from that block could we lose those events? Is it safer to go one down of the procotol id or am I over complicating this? Would have to see how FireFly acknowledges events from FFTM and maybe the assumption you have made is that you get all the events per block in the webscoket?

[EnriqueL8]

Ah you covered this already https://github.com/hyperledger/firefly/pull/1534/files#diff-eb673d841796eecd85e0b4f676fb8de03a03b31b2a7d66a5c63d600aab57e2dfR217
 sorry

[EnriqueL8]

Have tested this one E2E by doing the following experiment:

• Setup a 1.3.0 FF stack
• Deploy a contract and a listener with the option to listen from newest
• Delete the listener from EVMConnect
• Restart the FF with the code from this PR
• Find the new listener created in EVMConnect and make sure the fromBlock matches the last protocol ID for the last event - 1

[EnriqueL8]

This looks good to me!

[peterbroadhurst]

we recreate a listener with block number of the last protocol ID but we haven't consumed all the events from that block could we lose those events? Is it safer to go one down of the procotol id or am I over complicating this?

This is the reason we go one block back:

firefly/internal/blockchain/ethereum/eventstream.go

Lines 216 to 218 in 8dd5a0d

	// We jump back on block from the last event, to minimize re-delivery while ensuring
	// we get all events since the last delivered (including subsequent events in the same block)
	parsedUint--

[EnriqueL8]

Yeah I realised with #1534 (comment)