Conversation
l2geth/rollup/sync_service.go
Outdated
| } | ||
|
|
||
| // This reset function MUST be called on any error case | ||
| reset := func() { |
There was a problem hiding this comment.
maybe make this a defer to be robust against added failure modes in the future:
func applyTransactionToTip(tx *types.Transaction) (err error) {
defer func() {
if err != nil {
reset();
}
}()and then it can be moved up further.
🦋 Changeset detectedLatest commit: f4d1090 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
removed
the
C-protocol-critical
cfromknecht
left a comment
cfromknecht
left a comment
There was a problem hiding this comment.
Changes generally LGTM, want to first confirm that we should be resetting the enqueue index before approving tho
| } | ||
| } | ||
| return nil | ||
| s.SetLatestEnqueueIndex(queueIndex) |
There was a problem hiding this comment.
when the deadlock bug was fixed, there was a reason we decided to leave out resetting the enqueue index. i can't recall what the exact reason was, but wanted to bring it up so that we can revisit those assumptions
…RootAtTimestamp` (#2395) Closes #2383 , #2384 - Skipped testing for `supervisor_syncStatus` because that is being tested checking the chain progress. - Skipped `supervisor_dependencySetV1` because we need `QueryAPI` in Go implementation to implement the end-point so that we can test it using devstack. #### Changes - The serialize and deserialize implemented for `ChainID` is now generic for `u64` because I needed it at other places as well. - Due to the way Go unmarshalls the response, I needed to add the response objects `SuperRootOutputRpc` and `ChainRootInfoRpc`, which are essentially the same as `SuperRootOutput` and `ChainRootInfo` used before. The fields `timestamp`, `version` and `chainID` needed to be compatible with `hexutil.Uint64` and hexutil.Bytes`. Hence all the modifications. Tested locally, all tests including e2e tests are passing as expected.
4 participants
Description
Be sure to reset the values on error