fix: update hashing function to latest spec #64
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
The DR hashing function incorrectly includes
payback_address
andseda_payload
, which are not known by external chains at time of DR creation.Explanation of Changes
payback_address
andseda_payload
from DR hashing functionvalue
, keeping onlychain_id
andnonce
. This is only to reflect how memo is calculated in the EVM contracts, i.e. the CosmWasm contracts don't care about how memo is calculated.hash_update()
andpad_to_32_bytes()
since the hashing function no longer requires themTesting
test_hash_data_request()
to isolate the DR hashing function and to mirror an identical test on the EVM repo, see: https://github.com/sedaprotocol/seda-evm-contracts/blob/20fffd45abcdcd16e27c4f8ef0a4c343e0dbc371/test/SedaOracle.t.sol#L110Currently with the test inputs, the CosmWasm contracts and EVM contracts generate a test DR hash of
0x27e7eec2f319302826ea53ae08b4aac6c4824cc07eafcdaf740501b3d603d0aa
.Related PRs and Issues
Related to sedaprotocol/seda-evm-contracts#13
Created new issue #65 because Hash (
dr_binary_id
andtally_binary_id
) are currently Strings instead of fixed size byte arrays. Also requires the same update on the EVM side.