-
Notifications
You must be signed in to change notification settings - Fork 11.3k
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
[fastx adapter] introduce bytecode rewriter for module handle substit… #73
Conversation
3104ecc
to
cbe59cf
Compare
…ution This offers a solution to the bytecode rewriting problem explained in MystenLabs/sui#71. Like any substitution scheme, the logic is rather intricate, so I have included lots of comments, `debug_assert`s, and tests.
cbe59cf
to
5d7908e
Compare
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.
If there's something wrong with the below logic, it didn't jump off the page at me.
Co-authored-by: François Garillot <[email protected]>
Co-authored-by: François Garillot <[email protected]>
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.
LGTM!
(old_ids_for_friends.is_empty() && handle_index_sub_map.len() == self.sub_map.len()) | ||
|| handle_index_sub_map.len() <= self.sub_map.len() |
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.
Nit: I think what I was hinting at is that I suspect handle_index_sub_map.len() + old_ids_for_friends.len() = self.sub_map.len()
in all cases?
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.
Ah good question--this doesn't hold if module id I
is both a friend and in the module handles table. There's a test that exercises this scenario.
My invariant is a bit convoluted, but I think it's the strongest thing that's true.
…ution
This offers a solution to the bytecode rewriting problem explained in #71. Like any substitution scheme, the logic is rather intricate, so I have included lots of comments,
debug_assert
s, and tests.Once #72 lands, we can call this from the
publish
code path to resolve several TODO's (supporting multi-module publishing, stop unsafe clobbering ofself
address, ...).