Fix inner instruction events not being detected#4451
Conversation
|
@Matthias1590 is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
0x4ka5h
left a comment
There was a problem hiding this comment.
Prefer matching inner invoke with the same regex as the first log and only then push if the captured id equals this program. Also Could you please mirror the change in client/lib.rs and add a small parseLogs test there aswell.
|
Will do |
|
Requested changes are made. The added test failed pre-edit and passed afterwards, as expected. |
|
Formatting is fixed now |
|
Will this be merged? Constantly syncing my fork up to date for my projects is getting annoying :p |
|
Bump |
swaroop-osec
left a comment
There was a problem hiding this comment.
LGTM.
- Just needed tests as requested
@jamie-osec IMO we should also make these regexes statics with lazy init (e.g. OnceLock<Regex>), since right now handle_system_log recompiles two regexes on every log line and parse_logs_response / Execution::new each compile their own copy of effectively the same invoke pattern. Four Regex::new(...) call sites total, all the same anchored shape, all compiled on every call.
|
Would you mind if regexes get refactored in a separate pr? I can open one for that after this is merged but I'd like for this to be merged ASAP so I can start using actual releases in my projects again |
|
Updated the regexes, including some drive-by changes to other regexes in the same file. It feels like there's some redundancy with those, but fine for now |
|
Filed an issue for improving the test coverage |
Fixes #4450 by removing the "log:" part of the log check, if a log line starts with
Program ${programId.toString()}it belongs to programId, whether it's a log, or invoke.