This PR adds a safety check whenever looping through an InternalNode's children.
There are 2 parts to this PR:
1. Nil check
From my understanding, an InternalNode's child should be either the following: InternalNode, LeafNode, Empty, HashedNode, or UnknownNode. Hence, a child who is nil is not accepted, and should be considered an abnormal behavior. Based on the current codebase, the only place that can change an InternalNode's children to nil is the SetChild function. Hence, we should explicitly check for them and panic as soon as they are encountered.
2. Node type switch
There are plenty of places that do something like if _, ok := c.(Empty); !ok, which implies that any node type other than Empty is true. This isn't necessarily the true intention, because we also wouldn't want to deal with HashedNode and UnknownNode. Adding a node type switch ensures that we only want to deal with the correct node type (i.e. usually InternalNode and LeafNode only).
This PR adds a safety check whenever looping through an
InternalNode's children.There are 2 parts to this PR:
1. Nil check From my understanding, an
InternalNode's child should be either the following:InternalNode,LeafNode,Empty,HashedNode, orUnknownNode. Hence, a child who isnilis not accepted, and should be considered an abnormal behavior. Based on the current codebase, the only place that can change anInternalNode's children tonilis theSetChildfunction. Hence, we should explicitly check for them and panic as soon as they are encountered.2. Node type switch There are plenty of places that do something like
if _, ok := c.(Empty); !ok, which implies that any node type other thanEmptyis true. This isn't necessarily the true intention, because we also wouldn't want to deal withHashedNodeandUnknownNode. Adding a node type switch ensures that we only want to deal with the correct node type (i.e. usuallyInternalNodeandLeafNodeonly).