feat: SolEnum and SolInterface
#153
Conversation
DaniPopes
force-pushed
the
dani/sol-events
branch
from
8bb63bf
to
4eeb02a
Compare
crates/sol-types/src/types/calls.rs
Outdated
| Revert(Revert), | ||
| /// A panic. See [`Panic`] for more information. | ||
| Panic(Panic), | ||
| } |
There was a problem hiding this comment.
Should there be an empty variant? for data-less reverts
There was a problem hiding this comment.
This just kinda breaks the entire thing, since now we can't assume that data has a selector. I'm not sure how to do this, I think it's better we don't include this.
Same for fallback functions etc.
There was a problem hiding this comment.
yeah, I guess the question is whether we want this trait/enum to capture:
- ALL inputs and outputs (known ABI & unknown ABI & Empty)
- NON-EMPTY inputs and outputs (known ABI & unknown ABI)
- KNOWN inputs and outputs (known ABI & Empty)
- KNOWN & NON-EMPTY inputs and outputs (known ABI only)
Right now we have option 4. Do we have a plan for capturing the remainder?
There was a problem hiding this comment.
I'm thinking 4 is fine for now, maybe 3 in the future. I don't want to overload this PR
There was a problem hiding this comment.
how do we capture the other 2 categories using this trait? Something like this?
pub enum Revert<T: SolCalls >{
Known(T),
UnknownAbi(Vec<u8>)
Bare(Vec<u8>)
Empty
}
| @@ -87,7 +90,7 @@ pub enum ContractError<T> { | |||
| Panic(Panic), | |||
| } | |||
|
|
|||
| impl<T: SolCalls> SolCalls for ContractError<T> { | |||
| impl<T: SolInterface> SolInterface for ContractError<T> { | |||
There was a problem hiding this comment.
this allows accidentally mixing calls and errors, yeah?
ContractError<MyContractCalls> is valid, as is ContractError<MyContractErrors>
is there a way to fix this in the type system?
There was a problem hiding this comment.
Would either require a generic or assoc type which implements Sol*, but idk.
I'd also like to not have different traits for the event enum with something like type Selector = [u8; _] but not sure if doable.
I'd leave this for later since it's not that big of a deal.
There was a problem hiding this comment.
should we have a trait to capture this somehow?
trait SelectorPrefixed {}
trait SolError: SelectorPrefixed {}
that sort of thing?
| /// pub fn #is_variant,#as_variant,#as_variant_mut(...) -> ... { ... } | ||
| /// )* | ||
| /// } | ||
| /// ``` |
| } | ||
|
|
||
| fn expand_event(self, attrs: &[Attribute]) -> TokenStream { | ||
| // TODO: SolInterface for events |
There was a problem hiding this comment.
capture in an issue as this needs some conceptualizing I think? 🤔
Motivation
Closes #37
Closes #106
Solution
PR Checklist