Skip to content

Commit

Permalink
fix: parse selector hashes in sol macro (#730)
Browse files Browse the repository at this point in the history
* fix: parse selectors in sol macro

* run fmt

* add clippy lint suggestoins

* code formatting

* more formatting

* feat: include namespaces, add tests

* chore: use [u8; 4]

---------

Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com>
  • Loading branch information
cre-mer and DaniPopes committed Sep 6, 2024
1 parent 56ec91b commit 83e70b7
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 368 deletions.
81 changes: 80 additions & 1 deletion crates/sol-macro-expander/src/expand/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use proc_macro_error2::{abort, emit_error};
use quote::{format_ident, quote, TokenStreamExt};
use std::{
borrow::Borrow,
collections::HashMap,
fmt,
fmt::Write,
sync::atomic::{AtomicBool, Ordering},
};
Expand Down Expand Up @@ -157,7 +159,8 @@ impl<'ast> ExpCtxt<'ast> {

if !self.all_items.0.is_empty() {
self.resolve_custom_types();
if self.mk_overloads_map().is_err() {
// Selector collisions requires resolved types.
if self.mk_overloads_map().is_err() || self.check_selector_collisions().is_err() {
abort = true;
}
}
Expand Down Expand Up @@ -254,6 +257,82 @@ impl<'ast> ExpCtxt<'ast> {
}
}

/// Checks for function and error selector collisions in the resolved items.
fn check_selector_collisions(&mut self) -> std::result::Result<(), ()> {
#[derive(Clone, Copy)]
enum SelectorKind {
Function,
Error,
// We can ignore events since their selectors are 32 bytes which are unlikely to
// collide.
// Event,
}

impl fmt::Display for SelectorKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Function => "function",
Self::Error => "error",
// Self::Event => "event",
}
.fmt(f)
}
}

let mut result = Ok(());

let mut selectors = vec![HashMap::new(); 3];
let all_items = std::mem::take(&mut self.all_items);
for (namespace, items) in &all_items.0 {
self.with_namespace(namespace.clone(), |this| {
selectors.iter_mut().for_each(|s| s.clear());
for (_, &item) in items {
let (kind, selector) = match item {
Item::Function(function) => {
(SelectorKind::Function, this.function_selector(function))
}
Item::Error(error) => (SelectorKind::Error, this.error_selector(error)),
// Item::Event(event) => (SelectorKind::Event, this.event_selector(event)),
_ => continue,
};
let selector: [u8; 4] = selector.array.try_into().unwrap();
// 0x00000000 or 0xffffffff are reserved for custom errors.
if matches!(kind, SelectorKind::Error)
&& (selector == [0, 0, 0, 0] || selector == [0xff, 0xff, 0xff, 0xff])
{
emit_error!(
item.span(),
"{kind} selector `{}` is reserved",
hex::encode_prefixed(selector),
);
result = Err(());
continue;
}
match selectors[kind as usize].entry(selector) {
std::collections::hash_map::Entry::Vacant(entry) => {
entry.insert(item);
}
std::collections::hash_map::Entry::Occupied(entry) => {
result = Err(());
let other = *entry.get();
emit_error!(
item.span(),
"{kind} selector `{}` collides with `{}`",
hex::encode_prefixed(selector),
other.name().unwrap();

note = other.span() => "other declaration is here";
);
}
}
}
})
}
self.all_items = all_items;

result
}

fn mk_overloads_map(&mut self) -> std::result::Result<(), ()> {
let mut overloads_map = std::mem::take(&mut self.overloads);

Expand Down
38 changes: 38 additions & 0 deletions crates/sol-types/tests/ui/collisions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
use alloy_sol_types::sol;

// https://github.com/alloy-rs/core/issues/729

sol! {
error func_2093253501(bytes);
error transfer(address,uint256);

function func_2093253501(bytes);
function transfer(address,uint256);

error BlazingIt4490597615();

contract A {
error func_2093253501(bytes);
error transfer(address,uint256);

function func_2093253501(bytes);
function transfer(address,uint256);

error BlazingIt4490597615();
}
}

// This is OK.
mod namespaced {
use alloy_sol_types::sol;

sol! {
function func_2093253501(bytes);

contract B {
function transfer(address,uint256);
}
}
}

fn main() {}
35 changes: 35 additions & 0 deletions crates/sol-types/tests/ui/collisions.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: function selector `0xa9059cbb` collides with `func_2093253501`
--> tests/ui/collisions.rs:10:14
|
10 | function transfer(address,uint256);
| ^^^^^^^^
|
note: other declaration is here
--> tests/ui/collisions.rs:9:14
|
9 | function func_2093253501(bytes);
| ^^^^^^^^^^^^^^^

error: error selector `0x00000000` is reserved
--> tests/ui/collisions.rs:12:11
|
12 | error BlazingIt4490597615();
| ^^^^^^^^^^^^^^^^^^^

error: function selector `0xa9059cbb` collides with `func_2093253501`
--> tests/ui/collisions.rs:19:18
|
19 | function transfer(address,uint256);
| ^^^^^^^^
|
note: other declaration is here
--> tests/ui/collisions.rs:18:18
|
18 | function func_2093253501(bytes);
| ^^^^^^^^^^^^^^^

error: error selector `0x00000000` is reserved
--> tests/ui/collisions.rs:21:15
|
21 | error BlazingIt4490597615();
| ^^^^^^^^^^^^^^^^^^^
Loading

0 comments on commit 83e70b7

Please sign in to comment.