[Config Change] Removes HardReorg config from InboxReader #2542
Conversation
cla-bot
bot
added
the
s
diegoximenes
changed the title
| Require(t, err) | ||
|
|
||
| // Reorg out the user delayed message | ||
| err = tracker.ReorgDelayedTo(1, true) |
There was a problem hiding this comment.
Instead of just removing this test, we should instead have this test insert an alternative delayed message at position 1 which would cause the same reorg. Instead of getting the delayed count below, we could then get the delayed message to ensure it changed.
There was a problem hiding this comment.
I adjusted the test to trigger a reorg by adding an alternative delayed message through AddDelayedMessages as you mentioned, but the strategy that I implemented is a little bit different than the one you described.
diegoximenes
dismissed stale reviews from ganeshvanahalli and gligneul
via
94a07f0
diegoximenes
force-pushed
the
rm_hard_reorg
branch
from
0d584ee to
94a07f0
Compare
|
I think there is an issue in the inbox tracker that your test exposed. I pushed up a commit to this branch to fix it. Let me know what you think |
|
|
| @@ -49,7 +49,22 @@ func TestSequencerReorgFromDelayed(t *testing.T) { | |||
| }, | |||
| }, | |||
| } | |||
| err = tracker.AddDelayedMessages([]*DelayedInboxMessage{initMsgDelayed, userDelayed}, false) | |||
| delayedRequestId2 := common.BigToHash(common.Big2) | |||
| userDelayed2 := &DelayedInboxMessage{ | |||
There was a problem hiding this comment.
I really think it would improve the readability of these tests if we made a little helper method that could return a DelayedInboxMessage built from just the important arguments from the test case. For example, I think the only thing that matters here is that the first message uses delayedRequestId and the second one uses dealyedRequestId2, but I have to actually scan all of the lines between 38-51 and between 53-66 to be sure that something else doesn't differ between the two messages.
Also, if it weren't for your comment on line 137, I wouldn't have noticed that the timestamp was different. But, that would have been really clear if we used a helper function to get a "normal" DelayedInboxMessage and then modified the fields on the struct we wanted to change for the test.
Resolves NIT-1349
HardReorg config from InboxReader seems unused so this PR removes it.