[BUG] Fix resource ID encoding/decoded

[drewstone]

Overview

The resource ID encoding and decoding functions in primitives/src/utils is incorrect and does not match the derive_resource_id function that we are using primarily in Solidity. Instead of encoding bytes from the back it encodes the the TreeId as 20 bytes in the front of the ResourceId. This should be fixed.

The incorrectness of this implementation can also be seen simply by the comments as it states the first 20 bytes are the tree_id. This in fact is not consistent with how things are meant to be.

Current implementations

/// The ResourceId type is a 32 bytes array represented as the following:
/// ```md
/// +---+---+---+---+---+---+---+---+---+
/// | * |   |   |  ...  | * | * | * | * |
/// +-|-+---+---+---+---+-|-+---+---+---+
///   |                   +-> The last 4 bytes are the chain_id
///   +-> The first 20 bytes are the tree_id
/// ```
/// This takes the tree_id and the chain_id and combines them into a single 32
/// bytes array. the process is simple as convert the `tree_id` to 4 bytes array
/// (little-endian), pad the remaining `(20 - 4)` butes with zeros, next convert
/// the chain_id to 4 bytes array (little-endian) and append it to the last 4
/// bytes of the result array.
pub fn encode_resource_id<TreeId, ChainId>(tree_id: TreeId, chain_id: ChainId) -> ResourceId
where
	TreeId: Encode + Decode + AtLeast32Bit + Default + Copy,
	ChainId: Encode + Decode + AtLeast32Bit + Default + Copy,
{
	let mut result = [0u8; 32];
	let mut tree_id_bytes = tree_id.encode();
	tree_id_bytes.resize(20, 0); // fill the remaining 20 bytes with zeros
	let mut chain_id_bytes = chain_id.encode();
	chain_id_bytes.resize(4, 0); // fill the remaining 4 bytes with zeros

	debug_assert!(tree_id_bytes.len() == 20);
	debug_assert!(chain_id_bytes.len() == 4);
	result[0..20].copy_from_slice(&tree_id_bytes);
	result[28..].copy_from_slice(&chain_id_bytes);
	result
}

Correct implementation

pub fn derive_resource_id(chain: u32, id: &[u8]) -> ResourceId {
	let mut r_id: ResourceId = [0; 32];
	let chain = chain.to_le_bytes();
	// last 4 bytes of chain id,
	r_id[28] = chain[0];
	r_id[29] = chain[1];
	r_id[30] = chain[2];
	r_id[31] = chain[3];
	let range = if id.len() > 28 { 28 } else { id.len() }; // Use at most 28 bytes
	for i in 0..range {
		r_id[27 - i] = id[range - 1 - i]; // Ensure left padding for eth compatibility
	}
	r_id
}

[shekohex]

While I do agree that the encode_resource_id implementation is not correct, and should be the same as we do it in the EVM. I would suggest being as the following:

• Encoding: Big Endian.
• TreeId (mostly 4 Bytes) and should be left padded with zeros to fill the remaining 20 bytes to adhere to a ContractAddress (20 bytes).
• ChainId (should be 4 bytes) and it should be placed as the last x bytes (depending on its size, which we should agree on).

So the diagram in the comments is mostly correct, but the comments said it is Little Endian, which is not correct as we found out that EVM is using Big Endian, everywhere.