dymensionxyz / dymint

Sequencing Engine for Dymension RollApps
Other
98 stars 70 forks source link

fix: do not send duplicated consensus msgs in case of reboot #1222

Closed keruch closed 3 days ago

keruch commented 2 weeks ago

PR Standards


Close #1175

For Author:


For Reviewer:


After reviewer approval:

mtsitrin commented 1 week ago

@keruch don't we prefer to call SaveLastBlockSequencerSet only on changes instead of every block?

keruch commented 4 days ago

don't we prefer to call SaveLastBlockSequencerSet only on changes instead of every block?

@mtsitrin done!

keruch commented 4 days ago

What happens if the block H is created, applied and committed with a set S, but then the node crashes before SaveLastBlockSequencerSet?

The on boot the the set will be recomputed and put in consensus message queue, even though it was already included in the block H. So then it will be in H+1 too

Basically it seems to me that cons messages are inputs to a block. In our system, creating the block (using inputs) is not atomic with saving the result of the block ('outputs'). Here it's treated as output

Would another way to go about this be to just query the rollapp chain and compute the diff off of that?

@danwt the point is that UpsertSequencer duplication is not an erroneous case and its execution is idempotent. we are not required to protect against 100% edge cases, but just to prevent at least the most obvious spam (like sending too many consensus msgs in crash loop for example).

i don't see a way to do this process atomically, in every solution you have a "window" between getting the set from any source and saving it to the store. the source may be either memory as in my case, EndBlockerResponse if do it as RollappParamUpdates, or a rollapp query as you suggested.