Add test for network-specific hashing #6003
Comments
SWvheerden
pushed a commit
that referenced
this issue
Dec 8, 2023
Description --- Refactors the handling of network-based hashing operations introduced in #5980 to better handle read operations. Adds a sanity test for hash independence. Supersedes #6014. Closes #6003. Motivation and Context --- Recent work in #5980 binds the current network into consensus hashing. It uses a `Mutex`, which has two subtle issues. First, it allows the network to be set multiple times, which should not occur. Second, it locks on reads, which is unnecessary and inefficient. This PR updates how the current network is handled. It adds `Network::get_current_or_default` that will return either the current network (if it has been set) or the default network (if it has not). It adds `Network:set_current` that attempts to set the network; if it has been set before, this operation will fail. Note that calling `Network::get_current_or_default` does _not_ set the network for this purpose. It modifies the API for consensus hashing to add a wrapper that uses the current network. This wraps functionality allowing for specification of a network, which is useful for testing. On top of this new API, a new test is added that checks for distinct hashes using the same input but different networks. This is not comprehensive, but will detect obvious issues. How Has This Been Tested? --- Existing tests pass. A new test passes. What process can a PR reviewer use to test or verify this change? --- Check that the tests do what they claim. Check that the updates to consensus hashing properly introduce the expected wrapping functionality. Check that the updated network API does what it is supposed to.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Recent work in #5980 changes consensus hashing to bind to the network. It would be useful to add a sanity check that hashing the same input with different networks yields distinct hash outputs.
The text was updated successfully, but these errors were encountered: