-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix subcalls relation #321
Conversation
@@ -405,6 +405,10 @@ export function setUpItems(block: Block): void { | |||
rec.parentCall = prev | |||
populateSubcalls(prev, rec) | |||
} | |||
if (prev.parentCall && isSubcall(prev.parentCall, rec)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nested calls are handled incorrectly.
[1, 1, 1], [1, 1], [1] -> the first will be subcall of the second, the second will be subcall of the third
[1, 1, 1], [1] -> the first will be subcall of the second
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, i also expect that it works like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but i don't see the problem...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok if we talk about [1, 1, 1], [1]
then there will be wrong parent assignment...
the parent will have the child as a subcall but i believe [1]
shouldn't be in .parentCall
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk that should be the correct behavior here, but isSubcall
assumes that [1, 1, 1]
is a subcall of [1]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is expected according to subcalls query - https://github.com/subsquid/archive.py/blob/master/sqa/substrate/query.py#L422-L426
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is expected according to subcalls query
Not sure, I guess it only says "request all subcalls and thier children", but nothing about how they should be grouped
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, another way is to put only direct "children" into subcalls
array. i guess both options are fine...
@eldargab how subcalls are supposed to work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i got confirmation that subcalls represent all children
No description provided.