chore(customrawdb): unexport customrawdb.NewSyncPerformedIterator

[powerslider]

Why this should be merged

• This function is no longer needed by coreth so it can be unexported.

How this works

• Rename NewSyncPerformedIterator to newSyncPerformedIterator

How this was tested

existing UT

Need to be documented in RELEASES.md?

no

Signed-off-by: Tsvetan Dimitrov (tsvetan.dimitrov@avalabs.org)

[StephenButtolph]

Where is this needed in coreth?

[powerslider]

Where is this needed in coreth?

It is needed here https://github.com/ava-labs/coreth/blob/master/plugin/evm/vmtest/test_syncervm.go#L645, but instead of UnpackSyncPerfomedKey it is now called ParseSyncPerformedKey.

[StephenButtolph]

I feel like that test is very weird no? Why would that package test the internals of this package? Shouldn't it just assert the exported behavior of this package?

[powerslider]

I feel like that test is very weird no? Why would that package test the internals of this package? Shouldn't it just assert the exported behavior of this package?

True, it is used more in the sense of an additional assertion step here and here when testing the VM behavior so it is not in the scope of a unit test for this particular package. I personally don't mind removing it. Wdyt @StephenButtolph ?

[StephenButtolph]

True, it is used more in the sense of an additional assertion step here and here when testing the VM behavior so it is not in the scope of a unit test for this particular package. I personally don't mind removing it. Wdyt @StephenButtolph ?

Looking more closely, it seems like we should either expose both NewSyncPerformedIterator & parseSyncPerformedKey or neither.

I generally feel like reducing cross-package dependencies helps us more in the long-run...

Looking more closely at coreth, it seems that we currently use these to assert:

1. That the set is empty
2. The the set has a single entry

Couldn't we just use GetLatestSyncPerformed for these tests?

1. Verify that the result is 0
2. Verify that the result is equal to the provided entry

If we are okay with changing those tests to use GetLatestSyncPerformed, can we actually change this PR to unexport NewSyncPerformedIterator?

[powerslider]

If we are okay with changing those tests to use GetLatestSyncPerformed, can we actually change this PR to unexport NewSyncPerformedIterator?

Yes, absolutely! Now that you've mentioned it, I never actually realized that GetLatestSyncPerformed was doing the same thing as those two functions which is odd why it wasn't used in the first place.

In that case I'll unexport NewSyncPerformedIterator and I'll finish off the fix in coreth. Thanks!