Skip to content
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

Merged
merged 4 commits into from
Dec 20, 2021

Conversation

sblackshear
Copy link
Collaborator

@sblackshear sblackshear commented Dec 18, 2021

…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_asserts, 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 of self address, ...).

@sblackshear sblackshear force-pushed the bytecode_rewrite branch 2 times, most recently from 3104ecc to cbe59cf Compare December 19, 2021 00:51
@sblackshear sblackshear marked this pull request as ready for review December 19, 2021 00:53
…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.
Copy link
Contributor

@huitseeker huitseeker left a 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.

Copy link
Contributor

@huitseeker huitseeker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines +145 to +146
(old_ids_for_friends.is_empty() && handle_index_sub_map.len() == self.sub_map.len())
|| handle_index_sub_map.len() <= self.sub_map.len()
Copy link
Contributor

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?

Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[fastx programmability] Logic to update module handles of dependencies in module publishing flow
2 participants