EVM: revert data is not handled uniformly

[CedarMist]

Via: oasisprotocol/oasis-web3-gateway#429

When the EVM reverts it returns an arbitrary length array of bytes. When using Solidity this follows the convention of ABI encoding it with the signature Error(string). However, other custom errors can be returned, or a contract may decide to return arbitrary non ABI-encoded data for some reason.

Custom error example in Solidity:

    error Blah(uint);
    function testRevert () external view {
        revert Blah(123);
    }

The problem is the following code intercepts Error(string) encoded reverts and returns just the string. Otherwise it returns up to 1024 bytes of the revert data, as a base64 encoded string. This means it's not possible to know if the data returned is the raw revert data but base64 encoded, or a string extracted from the raw revert data.

The web3 gateway then tries to re-encode the error into the raw format returned from the EVM so it can be used by Ethereum clients which follow the ABI encoded error spec. This means, instead of getting a nice custom ABI encoded error type, you get Error(string) with a bunch of base64 encoded data - making custom errors difficult / annoying to use with Sapphire.

oasis-sdk/runtime-sdk/modules/evm/src/lib.rs

Lines 222 to 256 in 9573807

	// Decode revert reason, format is as follows:
	//
	// 08c379a0 <- Function selector
	// 0000000000000000000000000000000000000000000000000000000000000020 <- Offset of string return value
	// 0000000000000000000000000000000000000000000000000000000000000047 <- Length of string return value (the revert reason)
	// 6d7946756e6374696f6e206f6e6c79206163636570747320617267756d656e74 <- First 32 bytes of the revert reason
	// 7320776869636820617265206772656174686572207468616e206f7220657175 <- Next 32 bytes of the revert reason
	// 616c20746f203500000000000000000000000000000000000000000000000000 <- Last 7 bytes of the revert reason
	//
	const ERROR_STRING_SELECTOR: &[u8] = &[0x08, 0xc3, 0x79, 0xa0]; // Keccak256("Error(string)")
	const FIELD_OFFSET_START: usize = 4;
	const FIELD_LENGTH_START: usize = FIELD_OFFSET_START + 32;
	const FIELD_REASON_START: usize = FIELD_LENGTH_START + 32;
	const MIN_SIZE: usize = FIELD_REASON_START;
	const MAX_REASON_SIZE: usize = 1024;
	let max_raw_len = data.len().clamp(0, MAX_REASON_SIZE);
	if data.len() < MIN_SIZE || !data.starts_with(ERROR_STRING_SELECTOR) {
	return Err(Error::Reverted(base64::encode(&data[..max_raw_len])));
	}
	// Decode and validate length.
	let mut length =
	primitive_types::U256::from(&data[FIELD_LENGTH_START..FIELD_LENGTH_START + 32])
	.low_u32() as usize;
	if FIELD_REASON_START + length > data.len() {
	return Err(Error::Reverted(base64::encode(&data[..max_raw_len])));
	}
	// Make sure that this doesn't ever return huge reason values as this is at least
	// somewhat contract-controlled.
	if length > MAX_REASON_SIZE {
	length = MAX_REASON_SIZE;
	}
	let reason =
	String::from_utf8_lossy(&data[FIELD_REASON_START..FIELD_REASON_START + length]);
	Err(Error::Reverted(reason.to_string()))

While we can handle this entirely in web3-gateway in a clunky fashion, by detecting if the revert string is a valid base64 encoding and then return the custom error instead of wrapping it again in Error(string) encoding, there is overlap with valid revert strings and base64 data.

I suggest returning the raw revert data, base64 encoded, without doing custom parsing. Which removes the special case from both runtime-sdk/evm & oasis-web3-gateway. Existing reverts with string messages will continue to work as expected, and custom reverts will start working. But - this will cause a small amount of time when errors are broken until all the web3 gateways are upgraded after runtime-sdk.

Alternatively, we can do the clunky workaround, and add a workaround to a workaround (presumably the workaround was 'I want simple strings returned in error messages')