Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: RLP::decode() should return an empty string instead of "0" when decoding 0x80 #539

Merged
merged 8 commits into from
Nov 23, 2023
3 changes: 3 additions & 0 deletions crates/utils/src/errors.cairo
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// LENGTH
const RLP_EMPTY_INPUT: felt252 = 'KKT: EmptyInput';
const RLP_INPUT_TOO_SHORT: felt252 = 'KKT: InputTooShort';
const RLP_PAYLOAD_TOO_LONG: felt252 = 'KKT: PayloadTooLong';

#[derive(Drop, Copy, PartialEq)]
enum RLPError {
EmptyInput: felt252,
InputTooShort: felt252,
PayloadTooLong: felt252,
}


Expand All @@ -14,6 +16,7 @@ impl RLPErrorIntoU256 of Into<RLPError, u256> {
match self {
RLPError::EmptyInput(error_message) => error_message.into(),
RLPError::InputTooShort(error_message) => error_message.into(),
RLPError::PayloadTooLong(error_message) => error_message.into(),
}
}
}
Expand Down
22 changes: 19 additions & 3 deletions crates/utils/src/rlp.cairo
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use core::byte_array::ByteArrayTrait;
use core::result::ResultTrait;
use utils::errors::{RLPError, RLPHelpersError, RLP_EMPTY_INPUT, RLP_INPUT_TOO_SHORT};
use utils::errors::{
RLPError, RLPHelpersError, RLP_EMPTY_INPUT, RLP_INPUT_TOO_SHORT, RLP_PAYLOAD_TOO_LONG
};
use utils::helpers::{U32Trait, U256Impl, U128Impl, ArrayExtension};

// Possible RLP types
Expand Down Expand Up @@ -43,6 +45,9 @@ impl RLPImpl of RLPTrait {
Result::Ok((RLPType::String, 1, prefix_byte.into() - 0x80))
} else if prefix_byte < 0xc0 { // Long String
let len_bytes_count: u32 = (prefix_byte - 0xb7).into();
if len_bytes_count > 4 {
enitrat marked this conversation as resolved.
Show resolved Hide resolved
return Result::Err(RLPError::PayloadTooLong(RLP_PAYLOAD_TOO_LONG));
}
if input_len <= len_bytes_count {
return Result::Err(RLPError::InputTooShort(RLP_INPUT_TOO_SHORT));
}
Expand All @@ -54,6 +59,9 @@ impl RLPImpl of RLPTrait {
Result::Ok((RLPType::List, 1, prefix_byte.into() - 0xc0))
} else { // Long List
let len_bytes_count = prefix_byte.into() - 0xf7;
if len_bytes_count > 4 {
enitrat marked this conversation as resolved.
Show resolved Hide resolved
return Result::Err(RLPError::PayloadTooLong(RLP_PAYLOAD_TOO_LONG));
}
if input.len() <= len_bytes_count {
return Result::Err(RLPError::InputTooShort(RLP_INPUT_TOO_SHORT));
}
Expand Down Expand Up @@ -158,9 +166,9 @@ impl RLPImpl of RLPTrait {

match rlp_type {
RLPType::String => {
// checking for default value `0`
if (len == 0) {
output.append(RLPItem::String(array![0].span()));
// Empty strings are parsed as 0 for an integer
enitrat marked this conversation as resolved.
Show resolved Hide resolved
output.append(RLPItem::String(array![].span()));
} else {
output.append(RLPItem::String(input.slice(offset, len)));
}
Expand Down Expand Up @@ -192,6 +200,10 @@ impl RLPHelpersImpl of RLPHelpersTrait {
fn parse_u128_from_string(self: RLPItem) -> Result<u128, RLPHelpersError> {
match self {
RLPItem::String(bytes) => {
// Empty strings means 0
if bytes.len() == 0 {
return Result::Ok(0);
}
let value = U128Impl::from_bytes(bytes).ok_or(RLPHelpersError::FailedParsingU128)?;
Result::Ok(value)
},
Expand All @@ -202,6 +214,10 @@ impl RLPHelpersImpl of RLPHelpersTrait {
fn parse_u256_from_string(self: RLPItem) -> Result<u256, RLPHelpersError> {
match self {
RLPItem::String(bytes) => {
// Empty strings means 0
if bytes.len() == 0 {
return Result::Ok(0);
}
let value = U256Impl::from_bytes(bytes).ok_or(RLPHelpersError::FailedParsingU256)?;
Result::Ok(value)
},
Expand Down
6 changes: 3 additions & 3 deletions crates/utils/src/tests/test_data.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ fn legacy_rlp_encoded_tx() -> Span<u8> {
239,
1,
128,
128
128,
]
.span()
}
Expand Down Expand Up @@ -110,7 +110,7 @@ fn eip_2930_encoded_tx() -> Span<u8> {
171,
205,
239,
192
192,
]
.span()
}
Expand Down Expand Up @@ -169,7 +169,7 @@ fn eip_1559_encoded_tx() -> Span<u8> {
171,
205,
239,
192
192,
]
.span()
}
32 changes: 29 additions & 3 deletions crates/utils/src/tests/test_rlp.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use core::option::OptionTrait;
use core::traits::Into;

use result::ResultTrait;
use utils::errors::{RLPError, RLP_EMPTY_INPUT, RLP_INPUT_TOO_SHORT};
use utils::errors::{RLPError, RLP_EMPTY_INPUT, RLP_INPUT_TOO_SHORT, RLP_PAYLOAD_TOO_LONG};
use utils::rlp::{RLPType, RLPTrait, RLPItem};

// Tests source : https://github.com/HerodotusDev/cairo-lib/blob/main/src/encoding/tests/test_rlp.cairo
Expand Down Expand Up @@ -82,6 +82,32 @@ fn test_rlp_decode_type_long_list_len_too_short() {
);
}

#[test]
#[available_gas(99999999)]
fn test_rlp_decode_type_long_string_payload_too_long() {
let mut arr = array![0xbf, 0x01, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02, 0x02];

let res = RLPTrait::decode_type(arr.span());

assert(res.is_err(), 'Should have failed');
assert(
res.unwrap_err() == RLPError::PayloadTooLong(RLP_PAYLOAD_TOO_LONG), 'err != PayloadTooLong'
);
}

#[test]
#[available_gas(99999999)]
fn test_rlp_decode_type_long_list_payload_too_long() {
let mut arr = array![0xfc, 0x01, 0x02, 0x02, 0x02, 0x02];

let res = RLPTrait::decode_type(arr.span());

assert(res.is_err(), 'Should have failed');
assert(
res.unwrap_err() == RLPError::PayloadTooLong(RLP_PAYLOAD_TOO_LONG), 'err != PayloadTooLong'
);
}

#[test]
#[available_gas(9999999)]
fn test_rlp_empty() {
Expand Down Expand Up @@ -381,7 +407,7 @@ fn test_rlp_decode_string_default_value() {
let mut arr = array![0x80];

let rlp_item = RLPTrait::decode(arr.span()).unwrap();
let expected = RLPItem::String(array![0].span());
let expected = RLPItem::String(array![].span());

assert(rlp_item.len() == 1, 'item length not 1');
assert(*rlp_item[0] == expected, 'default value not 0');
Expand Down Expand Up @@ -2121,7 +2147,7 @@ fn test_rlp_decode_long_list() {
.span()
);

let mut expected_16 = RLPItem::String(array![0].span());
let mut expected_16 = RLPItem::String(array![].span());

let mut expected = array![
expected_0,
Expand Down
Loading