Closed haltman-at closed 1 year ago
Side note: After writing this, I had the thought -- hey, wait, can't we get rid of the searching by making use of the scope
field? Unfortunately no; it turns out that event definitions (and error definitions) don't include that. Dang.
Function definitions do, so notionally that could be used in #6050, but that would require putting them in referenceDeclarations
, and that would bog down that PR in naming disputes when I want to get it merged quickly. :P
So, Solidity 0.8.20 is out, and it changes which events get listed in a contract's ABI! That means our existing event allocation code won't work with 0.8.20 contracts. Fortunately, they also added the
usedEvents
property to contract nodes (much like the existingusedErrors
), which lets us distinguish when we're in this case, and also helps us handle it. So, now we have two event allocation pathways; one for pre-0.8.20, and one for post-0.8.20.In addition to this, there's also one change to the event decoding return type;
definedIn
can now benull
, as distinct fromundefined
(same as with errors). The use ofnull
here means that the event was defined at file level, as opposed toundefined
, which means we don't know where it was defined. Now you may say, but Solidity doesn't allow file-level events! Sure, but it will soon, and as long as I was doing this, I thought it made sense to future-proof for that. I've updated the event decoding inspector to account for this.Anyway, back to the allocation changes -- let's break down how this affects each of the three event allocation functions.
For
getEventAllocations
, it basically just means that inheritance handling is bypassed ifusedEvents
is present. Which is nice, because it means you get to skip the confusing part of the function! Well, OK, one of the confusing parts of the function.For
getEventAllocationsForContract
, it means we now have two separate allocation pathways... you'll notice this function now heavily parallelsgetReturndataAllocationsForContract
, this is not a coincidence. :P Note that the meaning ofundefined
differs between the two paths, though... in the old path cases ofundefined
are left in for inheritance processing, while in the new path they're filtered out since there won't be any.Finally
getEventAllocation
is the function whose changes you might find the most confusing, so let's go over that. First off, the function signature has changed; you can now pass in aneventNode
, since now we might already know that going in. We also introduce a new variable,definedInNode
; previously there was no need for this since the defined-in node was always the contract node, but now this is no longer the case! Ifnode
is defined butdefinedInNode
is not, that's interpreted as a file-level event.Anyway, you can see that if
eventNode
is not passed in, we run the old code, except that we setdefinedInNode
equal tocontractNode
. By contrast, ifeventNode
is passed in, then we run the new code, which thankfully is much simpler. It setsnode
toeventNode
(since we already know it, rather than having to find it like in the old case) and searches all the contracts to find the event we're looking for, settingdefinedInNode
as appropriate (or leaving it undefined if nothing was found -- this indicates a file-level event).(You may say, to determine if an event is file-level, shouldn't you search all the
SourceUnit
nodes to see if it's defined directly under one of them? Ideally, yes, but we don't have that information in the current setup and I didn't want to deal with changing that right now.)However, due to the special handling of library events, the new code does do one more check -- if it's a library event, and this contract isn't the library that defined it, we don't allocate it. Library events are always considered to be "in play", so we leave the event alone so its defining library can allocate it; we don't want it to appear twice, after all.
But that's it! Sorry, I know I'm modifying some confusing code here, but I hope you'll find that the code paths I've added are less confusing than the older existing code paths. :P
As for tests... I wanted to get this up quickly, so I didn't test it as thoroughly as I otherwise might have. I just updated the debugger and decoder tests to 0.8.20 and checked that they still passed. Hopefully that's enough!