Skip mandatory commitment when beefy authorities not changed

[yrong]

More context in #1137 authored by @Lederstrumpf and @bhargavbh, many thanks.

Revamp the relayer to skip mandatory commitment when beefy authorities not changed.

[yrong]

Sync beefy checkpoint from 20733663 to the most recent on polkadot, all work as expected.

Log as follows shows all handover commitments from 20919030 to 20926196(range of 3 sessions) are skipped since no authorities changes.

...
{"@timestamp":"2024-05-28T18:38:01.185131892Z","IsHandover":true,"blockNumber":20919030,"level":"debug","message":"Transaction SubmitFinal succeeded","tx":"0x36e0dc7787b3aeb873d278816d3a3ca20dfd7fd3965045aa5310a5ed920ccf7e"}
...
{"@timestamp":"2024-05-28T18:38:01.186696501Z","ValidatorSetID":575,"blockNumber":20921420,"level":"warning","message":"Ignore mandatory commitment authorities not change"}
...
{"@timestamp":"2024-05-28T18:41:14.533439186Z","IsHandover":true,"blockNumber":20926196,"level":"debug","message":"Transaction SubmitFinal succeeded","tx":"0xe89092abf8d7c0f7c94fefdb50c82bb6f36ebd9deaa0bef632908799ee333920"}

Based upon the analysis #1137 (comment) will decrease the maintainance cost about 4 times less.

[alistair-singh]

We need some tests in the solidity that skips a mandatory commitment.

Example scenario:
We are in validator set id 6 (next validator set 7), we skip 7 and 8 and then import 9. After importing 9 the current validator set will be 7 and the next validator set will be 10.

See here: https://github.com/Snowfork/snowbridge/pull/1215/files#diff-473fead6cd500a4a05f06d0cea4d9ef166c9bdbe2a2e1a06a33d2e1aedcc428eR379-R382

We also need to make sure that a future commit does not commit to a block number in the past and a test that covers it.

Context:
#1137 (comment)
#1137 (comment)

[vgeddes]

Good catch Alistair, yeah, we need more unit tests for this work.

[yrong]

Yeah, so in this case the current&next validatorId pair will be (7,10) and latestBeefyBlock is in session 9.

I would expect a minor change here

snowbridge/contracts/src/BeefyClient.sol

Line 259 in 40f9592

if (commitment.validatorSetID == currentValidatorSet.id) {

from if (commitment.validatorSetID == currentValidatorSet.id) to if (commitment.validatorSetID == currentValidatorSet.id) || commitment.validatorSetID == nextValidatorSet.id - 1 will just work but I'll definetely do more tests.

[yrong]

c11ed7d

[vgeddes]

@yrong @alistair-singh One thing to consider in the greater scheme of things:

This optimization will greatly reduce costs for long-range catch up if the relayer is out of action for a few days. However, if my thinking is correct, it won't actually reduce costs if the relayer is caught up, and users are actively sending messages to Ethereum, because, we still need to update the MMR root so that the Gateway can verify user messages. And in order to update the MMR root, we need to sync a signed commitment that can be used to verify the new MMR root.

I still think its a worthwhile change to include, but we need more testing to be certain.

[yrong]

However, if my thinking is correct, it won't actually reduce costs if the relayer is caught up, and users are actively sending messages to Ethereum, because, we still need to update the MMR root so that the Gateway can verify user messages. And in order to update the MMR root, we need to sync a signed commitment that can be used to verify the new MMR root.

This PR is not only for long-range catch up case, it's supposed to work together with #1217 to furthurly decrease the cost. Here is my test flow based on polkadot relaychain.

1. initialize beefy checkpoint at 21022400, the current&next validatorSetID pair is (617,618).

2. sync mandatory commitment to 21026603, as log shows since there is no authorities change in validatorSetID 619 so the intermediate block 21024214 is ignored and after that the current&next validatorSetID pair in LC becomes (618,620) into the non-consecutive mode.

{"@timestamp":"2024-06-01T17:08:23.644984414Z","IsHandover":true,"commitment":{"blockNumber":21024214,"nextValidatorSetID":619,"validatorSetID":618},"lastSyncedBeefyBlock":21022400,"level":"info","message":"Discarded commitment with beefy authorities not change","validatorSetID":617}
...
{"@timestamp":"2024-06-01T17:09:24.131435105Z","IsHandover":true,"blockNumber":21026603,"level":"debug","message":"Transaction SubmitFinal succeeded","tx":"0xf9e0b6f99156a101be562ceda8a37eb0bd9cbf51ea64dfd704e358e184c10fcd"}

3. launch the one-shot update for the next block 21026604(i.e. a non-handover commitment in validatorSetID 619)

snowbridge-relay sync-latest-beefy-commitment --private-key *** --relay-block 21026604
{"@timestamp":"2024-06-01T17:11:06.260172937Z","blockNumber":21026611,"level":"debug","message":"Transaction SubmitFinal succeeded","tx":"0xafa01ea1c014f72d991f61eb4ac64fedba21dc2f523b5ba4dc8cba6ce8e51ced"}
{"@timestamp":"2024-06-01T17:11:06.261349409Z","currentValidatorSetID":618,"latestBeefyBlock":21026611,"level":"info","message":"Sync beefy update success","nextValidatorSetID":620}

As we can see the sync is success because the authorities in validatorSetID 619 are the same as 618 so it's possible to use the keys in validatorSetID 618 to verify the commitment in validatorSetID 619, and after that the current&next validatorSetID pair is still (618,620).

4. launch the one-shot update for the block 21029004(i.e. a handover commitment in validatorSetID 620)

snowbridge-relay sync-latest-beefy-commitment --private-key *** --relay-block 21029004
{"@timestamp":"2024-06-01T17:28:21.13045321Z","blockNumber":21029007,"level":"debug","message":"Transaction SubmitFinal succeeded","tx":"0x0824d2f0ab33b8647ec519afed3790cab902186d6738313bf239d95d52180b07"}
{"@timestamp":"2024-06-01T17:28:21.131650355Z","currentValidatorSetID":620,"latestBeefyBlock":21029007,"level":"info","message":"Sync beefy update success","nextValidatorSetID":621}

As we can see after that the current&next validatorSetID pair becomes (620,621) which is back to normal as before.