Skip to content

Commit

Permalink
Polkadot: Reverts return encoded error data (#1449)
Browse files Browse the repository at this point in the history
This is a continuation of #1415. `require()`, `assert()` and `revert()`
now return error data, according to the [Ethereum Solidity
documentation](https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require).
Additionally, many reverts inserted by the compiler now return the
corresponding `Panic(uint256)` error data, to align Solang closer with
`solc`.

The error types known to the contract are added in the metadata
`lang_error` field. At the moment there are only `Error` and `Panic`
because we don't support custom errors yet.

Refactored revert-related code into a dedicated `codegen` module.
Refactored the `polkadot::errors` into distinct tests, made them less
brittle and added assertions for the execution output.

This works allows implementing a follow-up PR for bubbling up uncaught exceptions
(this is why it's already included that in the documentation).

---------

Signed-off-by: xermicus <[email protected]>
  • Loading branch information
xermicus authored Aug 10, 2023
1 parent 586bb3c commit bac5707
Show file tree
Hide file tree
Showing 22 changed files with 1,342 additions and 613 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ wasm-encoder = "0.29"
toml = "0.7"
wasm-opt = { version = "0.112.0", optional = true }
contract-build = { version = "3.0.1", optional = true }
primitive-types = { version = "0.12", features = ["codec"] }


[dev-dependencies]
num-derive = "0.4"
primitive-types = { version = "0.12", features = ["codec"] }
wasmi = "0.30"
# rand version 0.7 is needed for ed25519_dalek::keypair::generate, used in solana_tests/signature_verify.rs
rand_07 = { package = "rand", version = "0.7" }
Expand Down
68 changes: 67 additions & 1 deletion docs/targets/polkadot.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ Solidity flavored for the Polkadot target has the following differences to Ether
- Constructors can be named. Constructors with no name will be called ``new`` in the generated metadata.
- There is no ``ecrecover()`` builtin function, or any other function to recover or verify cryptographic signatures at runtime
- Only functions called via rpc may return values; when calling a function in a transaction, the return values cannot be accessed
- An `assert()`, `require()`, or `revert()` executes the wasm unreachable instruction. The reason code is lost

There is a solidity example which can be found in the
`examples <https://github.com/hyperledger/solang/tree/main/examples>`_
Expand Down Expand Up @@ -62,3 +61,70 @@ The following example shows how call flags can be used:
.. include:: ../examples/polkadot/call_flags.sol
:code: solidity


Reverts and error data decoding
_______________________________

When a contract reverts, the returned error data is what the
`EVM would return <https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require>`_.
``assert()``, ``require()``, or ``revert()`` will revert the contract execution, where the revert reason (if any) is
encoded as ``Error(string)`` and provided in the execution output. Solidity contracts can also revert with a ``Panic(uint256)``
(please refer to the
`Ethereum Solidity language documentation <https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require>`_
for more information about when ``Panic`` might be returned).
Uncaught exceptions from calling and instantiating contracts or transferring funds will be bubbled
up back to the caller.

The metadata contains all error variants that the contract `knows` about in the ``lang_error`` field.

.. warning::

Never trust the error data.

Solidity contracts do bubble up uncaught errors. This can lead to situations where the
contract reverts with error data unknown to the contracts. Examples of this include
bubbling up custom error data from the callee or error data from an ``ink!`` contract.

The 4 bytes selector of the error data can be seen as the enum discriminator or index. However,
because SCALE encoding does not allow index larger than 1 byte, the hex-encoded error selector
is provided as the path of the error variant type in the metadata.

In the following example, the ``Panic`` variant of ``lang_error`` is of type ``10``, which looks like this:

.. code-block:: json
{
"id": 10,
"type": {
"def": {
"composite": {
"fields": [
{
"type": 9
}
]
}
},
"path": [
"0x4e487b71"
]
}
}
From this follows that error data matching the ``Panic`` selector of `0x4e487b71` can be decoded
according to type ``10`` (where the decoder must exclude the first 4 selector bytes).

.. note::

Ethereum Solidity knows about ``Error``, ``Panic`` and
`custom errors <https://docs.soliditylang.org/en/latest/abi-spec.html#errors>`_.
Solang does not yet support custom errors. For now, only ``Error`` (selector of `0x08c379a0`)
and ``Panic`` (selector of `0x4e487b71`) are returned and occur in the metadata.

The general process of decoding the output data of Solang Solidity contracts is as follows:

1. The compiler of the contract must be Solang (check the ``compiler`` field in the contract metadata).
2. If the revert flag is **not** set, the contract didn't revert and the output should be decoded as specified in the message spec.
3. If the output length is smaller than 4 bytes, the error data can't be decoded (contracts may return empty error data, for example if ``revert()`` without arguments is used).
4. If the first 4 bytes of the output do **not** match any of the selectors found in ``lang_error``, the error can't be decoded.
5. **Skip** the selector (first 4 bytes) and decode the remaining data according to the matching type found in `lang_error`.
4 changes: 4 additions & 0 deletions integration/polkadot/asserts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ describe('Deploy asserts contract and test', () => {
expect(res1.result.asOk.flags.isRevert).toStrictEqual(true);
expect(res1.result.asOk.data.toString()).toStrictEqual("0x08c379a0204920726566757365");

res1 = await query(conn, alice, contract, "testAssert");
expect(res1.result.asOk.flags.isRevert).toStrictEqual(true);
expect(res1.result.asOk.data.toString()).toStrictEqual("0x4e487b710100000000000000000000000000000000000000000000000000000000000000");

let gasLimit = await weight(conn, contract, "testAssert");
let tx = contract.tx.testAssert({ gasLimit });

Expand Down
52 changes: 44 additions & 8 deletions src/abi/polkadot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ fn primitive_to_ty(ty: &ast::Type, registry: &mut PortableRegistryBuilder) -> u3

fn int_to_ty(ty: &ast::Type, registry: &mut PortableRegistryBuilder) -> u32 {
let (signed, scalety) = match ty {
ast::Type::Uint(n) => ('u', n.next_power_of_two()),
ast::Type::Int(n) => ('i', n.next_power_of_two()),
ast::Type::Uint(n) => ("uint", n.next_power_of_two()),
ast::Type::Int(n) => ("int", n.next_power_of_two()),
_ => unreachable!(),
};
let def = match (signed, scalety) {
('u', n) => match n {
("uint", n) => match n {
8 => TypeDefPrimitive::U8,
16 => TypeDefPrimitive::U16,
32 => TypeDefPrimitive::U32,
Expand All @@ -66,25 +66,55 @@ fn int_to_ty(ty: &ast::Type, registry: &mut PortableRegistryBuilder) -> u32 {
256 => TypeDefPrimitive::U256,
_ => unreachable!(),
},
('i', n) => match n {
("int", n) => match n {
8 => TypeDefPrimitive::I8,
16 => TypeDefPrimitive::I16,
32 => TypeDefPrimitive::I32,
64 => TypeDefPrimitive::I64,
128 => TypeDefPrimitive::I128,
256 => TypeDefPrimitive::I256,

_ => unreachable!(),
},
_ => {
unreachable!()
}
_ => unreachable!(),
};
let path = path!(format!("{signed}{scalety}"));
let ty = Type::new(path, vec![], TypeDef::Primitive(def), Default::default());
registry.register_type(ty)
}

/// Build the `lang_error` type of this contract, where `errors` is a list
/// containing each error's name, selector and types. Returns a `TypeSpec`
/// of `TypeDefVariant` with each error as a variant.
fn lang_error(
ns: &ast::Namespace,
reg: &mut PortableRegistryBuilder,
errors: &[(&str, u32, Vec<ast::Type>)],
) -> TypeSpec<PortableForm> {
let variants = errors.iter().enumerate().map(|(n, (name, selector, ty))| {
let struct_fields = ty
.iter()
.map(|ty| resolve_ast(ty, ns, reg).into())
.map(|field| Field::new(None, field, None, Default::default()))
.collect::<Vec<_>>();
let ty = Type::new(
path!(format!("0x{}", hex::encode(selector.to_be_bytes()))),
vec![],
TypeDef::Composite(TypeDefComposite::new(struct_fields)),
Default::default(),
);
Variant {
name: name.to_string(),
fields: vec![Field::new(None, reg.register_type(ty).into(), None, vec![])],
index: n.try_into().expect("we do not allow custome error types"),
docs: Default::default(),
}
});
let type_def = TypeDefVariant::new(variants);
let path = path!("SolidityError");
let id = reg.register_type(Type::new(path.clone(), vec![], type_def, vec![]));
TypeSpec::new(id.into(), path)
}

/// Given an `ast::Type`, find and register the `scale_info::Type` definition in the registry
fn resolve_ast(ty: &ast::Type, ns: &ast::Namespace, registry: &mut PortableRegistryBuilder) -> u32 {
match ty {
Expand Down Expand Up @@ -471,12 +501,18 @@ pub fn gen_project(contract_no: usize, ns: &ast::Namespace) -> InkProject {
))
.done();

let error_definitions = &[
("Error", 0x08c379a0, vec![ast::Type::String]),
("Panic", 0x4e487b71, vec![ast::Type::Uint(256)]),
];

let spec = ContractSpec::new()
.constructors(constructors)
.messages(messages)
.events(events)
.docs(vec![render(&ns.contracts[contract_no].tags)])
.environment(environment)
.lang_error(lang_error(ns, &mut registry, error_definitions))
.done();

InkProject::new_portable(storage, spec, registry.finish())
Expand Down
2 changes: 1 addition & 1 deletion src/codegen/dispatch/polkadot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::{
codegen::{
cfg::{ASTFunction, ControlFlowGraph, Instr, InternalCallTy, ReturnCode},
encoding::{abi_decode, abi_encode},
expression::log_runtime_error,
revert::log_runtime_error,
vartable::Vartable,
Builtin, Expression, Options,
},
Expand Down
8 changes: 5 additions & 3 deletions src/codegen/encoding/buffer_validator.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::codegen::cfg::{ControlFlowGraph, Instr};
use crate::codegen::expression::assert_failure;
use crate::codegen::revert::{assert_failure, PanicCode, SolidityError};
use crate::codegen::vartable::Vartable;
use crate::codegen::Expression;
use crate::sema::ast::{Namespace, Type};
Expand Down Expand Up @@ -143,7 +143,8 @@ impl BufferValidator<'_> {
cfg.set_basic_block(invalid);

// TODO: This needs a proper error message
assert_failure(&Loc::Codegen, None, ns, cfg, vartab);
let error = SolidityError::Panic(PanicCode::Generic);
assert_failure(&Loc::Codegen, error, ns, cfg, vartab);

cfg.set_basic_block(valid);
}
Expand Down Expand Up @@ -222,7 +223,8 @@ impl BufferValidator<'_> {

cfg.set_basic_block(out_of_bounds_block);
// TODO: Add an error message here
assert_failure(&Loc::Codegen, None, ns, cfg, vartab);
let error = SolidityError::Panic(PanicCode::Generic);
assert_failure(&Loc::Codegen, error, ns, cfg, vartab);
cfg.set_basic_block(inbounds_block);
}

Expand Down
13 changes: 10 additions & 3 deletions src/codegen/encoding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
/// Any such helper function should work fine regardless of the encoding scheme being used.
mod borsh_encoding;
mod buffer_validator;
mod scale_encoding;
pub(super) mod scale_encoding;

use crate::codegen::cfg::{ControlFlowGraph, Instr};
use crate::codegen::encoding::borsh_encoding::BorshEncoding;
Expand Down Expand Up @@ -180,7 +180,7 @@ fn calculate_size_args(
/// However, this might be less suitable for schemas vastly different than SCALE or Borsh.
/// In the worst case scenario, you need to provide your own implementation of `fn encode(..)`,
/// which effectively means implementing the encoding logic for any given sema `Type` on your own.
pub(super) trait AbiEncoding {
pub(crate) trait AbiEncoding {
/// The width (in bits) used in size hints for dynamic size types.
fn size_width(
&self,
Expand Down Expand Up @@ -1732,10 +1732,17 @@ pub(super) trait AbiEncoding {

/// Returns if the we are packed encoding
fn is_packed(&self) -> bool;

/// Encode constant data at compile time.
///
/// Returns `None` if the data can not be encoded at compile time.
fn const_encode(&self, _args: &[Expression]) -> Option<Vec<u8>> {
None
}
}

/// This function should return the correct encoder, given the target
pub(super) fn create_encoder(ns: &Namespace, packed: bool) -> Box<dyn AbiEncoding> {
pub(crate) fn create_encoder(ns: &Namespace, packed: bool) -> Box<dyn AbiEncoding> {
match &ns.target {
Target::Solana => Box::new(BorshEncoding::new(packed)),
// Solana utilizes Borsh encoding and Polkadot, SCALE encoding.
Expand Down
112 changes: 112 additions & 0 deletions src/codegen/encoding/scale_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ use crate::codegen::vartable::Vartable;
use crate::codegen::{Builtin, Expression};
use crate::sema::ast::StructType;
use crate::sema::ast::{Namespace, Type, Type::Uint};
use parity_scale_codec::Encode;
use primitive_types::U256;
use solang_parser::pt::Loc::Codegen;
use std::collections::HashMap;

Expand Down Expand Up @@ -578,4 +580,114 @@ impl AbiEncoding for ScaleEncoding {
fn is_packed(&self) -> bool {
self.packed_encoder
}

/// TODO: This is used and tested for error data (Error and Panic) only.
fn const_encode(&self, args: &[Expression]) -> Option<Vec<u8>> {
let mut result = vec![];
for arg in args {
match arg {
Expression::AllocDynamicBytes {
initializer: Some(data),
ty: Type::String | Type::DynamicBytes,
..
} => result.extend_from_slice(&data.encode()),
Expression::AllocDynamicBytes {
initializer: Some(data),
ty: Type::Slice(inner),
..
} if matches!(**inner, Type::Bytes(1)) => result.extend_from_slice(data),
Expression::NumberLiteral {
ty: Type::Bytes(4),
value,
..
} => {
let bytes = value.to_bytes_be().1;
if bytes.len() < 4 {
let mut buf = Vec::new();
buf.resize(4 - bytes.len(), 0);
result.extend_from_slice(&buf);
}
result.extend_from_slice(&bytes[..]);
}
Expression::NumberLiteral {
ty: Type::Uint(256),
value,
..
} => {
let bytes = value.to_bytes_be().1;
result.extend_from_slice(&U256::from_big_endian(&bytes).encode()[..]);
}
_ => return None,
}
}
result.into()
}
}

#[cfg(test)]
mod tests {
use num_bigint::{BigInt, Sign};
use parity_scale_codec::Encode;
use primitive_types::U256;

use crate::{
codegen::{
encoding::{scale_encoding::ScaleEncoding, AbiEncoding},
Expression,
},
sema::ast::Type,
};

#[test]
fn const_encode_dynamic_bytes() {
let data = vec![0x41, 0x41];
let encoder = ScaleEncoding::new(false);
let expr = Expression::AllocDynamicBytes {
loc: Default::default(),
ty: Type::DynamicBytes,
size: Expression::Poison.into(),
initializer: data.clone().into(),
};
let encoded = encoder.const_encode(&[expr]).unwrap();
assert_eq!(encoded, data.encode());
}

#[test]
fn const_encode_uint() {
let encoder = ScaleEncoding::new(false);
for value in [U256::MAX, U256::zero(), U256::one()] {
let mut bytes = [0u8; 32].to_vec();
value.to_big_endian(&mut bytes);
let data = BigInt::from_bytes_be(Sign::Plus, &bytes);
let expr = Expression::NumberLiteral {
loc: Default::default(),
ty: Type::Uint(256),
value: data,
};
let encoded = encoder.const_encode(&[expr]).unwrap();
assert_eq!(encoded, value.encode());
}
}

#[test]
fn const_encode_bytes4() {
let encoder = ScaleEncoding::new(false);
for value in [
[0x00, 0x00, 0xff, 0xff],
[0x00, 0xff, 0xff, 0x00],
[0xff, 0xff, 0x00, 0x00],
[0xff, 0xff, 0xff, 0xff],
[0x00, 0x00, 0x00, 0x00],
[0xde, 0xad, 0xbe, 0xef],
[0x01, 0x00, 0x00, 0x00],
[0x00, 0x00, 0x00, 0x01],
] {
let expr = Expression::NumberLiteral {
ty: Type::Bytes(4),
value: BigInt::from_bytes_be(Sign::Plus, &value),
loc: Default::default(),
};
assert_eq!(&encoder.const_encode(&[expr]).unwrap(), &value.encode());
}
}
}
Loading

0 comments on commit bac5707

Please sign in to comment.