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

Significant dyn-abi fixes :) #168

Merged
merged 17 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions crates/dyn-abi/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,25 +184,49 @@ impl<'a> TryFrom<&'a str> for TupleSpecifier<'a> {
// flexible for `a, b` or `(a, b)`, or `tuple(a, b)`
// or any missing parenthesis
let value = value.trim();
let value = value.strip_suffix(')').unwrap_or(value);
let value = value.strip_prefix("tuple").unwrap_or(value);
let value = value.strip_prefix('(').unwrap_or(value);

// if we strip a trailing paren we MUST strip a leading paren
let value = if let Some(val) = value.strip_suffix(')') {
val.strip_prefix("tuple")
.unwrap_or(val)
.strip_prefix('(')
.ok_or_else(|| DynAbiError::invalid_type_string(value))?
} else {
value
};

let value = value
.strip_suffix(')')
.and_then(|val| val.strip_prefix('('))
.and_then(|val| val.strip_prefix("tuple"))
.unwrap_or(value);
prestwich marked this conversation as resolved.
Show resolved Hide resolved

// passes over nested tuples
let mut types = vec![];
let mut types: Vec<TypeSpecifier<'_>> = vec![];
let mut start = 0;
let mut depth = 0;
let mut depth: usize = 0;
for (i, c) in value.char_indices() {
match c {
'(' => depth += 1,
')' => depth -= 1,
')' => {
// handle extra closing paren
depth = depth
.checked_sub(1)
.ok_or_else(|| DynAbiError::invalid_type_string(value))?;
}
',' if depth == 0 => {
types.push(value[start..i].try_into()?);
start = i + 1;
}
_ => {}
}
}

// handle extra open paren
if depth != 0 {
return Err(DynAbiError::invalid_type_string(value))
}

// handle termina commas in tuples
prestwich marked this conversation as resolved.
Show resolved Hide resolved
let candidate = value[start..].trim();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a bug. for (T,) it would include the blank string, which would fail parsing into a typespecifier

if !candidate.is_empty() {
Expand Down Expand Up @@ -354,6 +378,24 @@ impl core::str::FromStr for DynSolType {
mod tests {
use super::*;

#[test]
fn extra_close_parens() {
let test_str = "bool,uint256))";
assert_eq!(
parse(test_str),
Err(DynAbiError::invalid_type_string(test_str))
);
}

#[test]
fn extra_open_parents() {
let test_str = "(bool,uint256";
assert_eq!(
parse(test_str),
Err(DynAbiError::invalid_type_string(test_str))
);
}

#[test]
fn it_parses_tuples() {
assert_eq!(
Expand Down
9 changes: 7 additions & 2 deletions crates/dyn-abi/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ impl<'a> DynToken<'a> {

/// Decodes from a decoder, populating the structure with the decoded data.
#[inline]
pub fn decode_populate(&mut self, dec: &mut Decoder<'a>) -> Result<()> {
pub(crate) fn decode_populate(&mut self, dec: &mut Decoder<'a>) -> Result<()> {
let dynamic = self.is_dynamic();
match self {
Self::Word(w) => *w = WordToken::decode_from(dec)?.0,
Expand All @@ -157,7 +157,12 @@ impl<'a> DynToken<'a> {
let mut child = child.raw_child();

let mut new_tokens: Vec<_> = Vec::with_capacity(size);
new_tokens.resize(size, *(template.take().unwrap()));
// This expect is safe because this is only invoked after
// `empty_dyn_token()` which always sets template
let t = template
.take()
.expect("No template. This is an alloy bug. Please report it.");
new_tokens.resize(size, *t);

new_tokens
.iter_mut()
Expand Down