Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

orderwatch: Remove null txHashes & fix uninstantiated map issue #280

Merged
merged 3 commits into from Jul 18, 2019

Conversation

@fabioberger
Copy link
Contributor

fabioberger commented Jul 18, 2019

During testing I noticed that OrderEvents consistently had 2 txHashes, with the first one always being the null address. By inspecting the code, I saw we are appending to an array created using make() which pre-created len number of zero-value entries. I've decided to fix by reverting to using []common.Hash{} instead of make(). Although less efficient, in the subsequent looping over the map, we don't have a readily available index to use in replacing the zero-values and so this feels like the cleanest approach.

I additionally noticed we weren't instantiating TxHashes in the cleanup job, which would have caused later code to attempt to treat a nil value as an instantiated map.

fabioberger added 2 commits Jul 18, 2019
…pending to this leaves the zero-value entries which is not what we want
@albrow albrow self-requested a review Jul 18, 2019
@albrow
albrow approved these changes Jul 18, 2019
Copy link
Member

albrow left a comment

Approved assuming CI passes.

@fabioberger fabioberger merged commit 7b19d88 into development Jul 18, 2019
1 check passed
1 check passed
ci/circleci: build Your tests passed on CircleCI!
Details
@fabioberger fabioberger deleted the fix/removeNullTxAddress branch Jul 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.