Skip to content
This repository has been archived by the owner on Mar 20, 2024. It is now read-only.

Rebase onto Sui #396

Open
brson opened this issue Dec 1, 2023 · 10 comments
Open

Rebase onto Sui #396

brson opened this issue Dec 1, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@brson
Copy link
Collaborator

brson commented Dec 1, 2023

We think the Sui storage model will be easier to implement on Solana than the mainline Move storage model. Let's attempt to rebase onto their tree.

@jcivlin Do you want to try this or should I?

@brson brson added the enhancement New feature or request label Dec 1, 2023
@jcivlin
Copy link
Collaborator

jcivlin commented Dec 1, 2023

I'm deep in dwarf riddle, plz go ahead. Let me know if you need help.

We think the Sui storage model will be easier to implement on Solana than the mainline Move storage model. Let's attempt to rebase onto their tree.

@jcivlin Do you want to try this or should I?

@brson I'm brain breaking with the dwarf riddle, plz go ahead. Let me know if you need help.

@brson
Copy link
Collaborator Author

brson commented Dec 11, 2023

The state of sui move is discouraging.

Their Move codebase is part of the sui tree, and the Move subdirs only slightly resemble the upstream Move code organization:

https://github.com/MystenLabs/sui/tree/main/external-crates/move

The sui-move branch of the Move repo hasn't been updated for 9 months, and I'm guessing won't be updated again:

https://github.com/move-language/move/tree/sui-move

Mysten doesn't even have a fork of the move repo under their org, so it seems like they are just maintaining their fork in the sui tree.


One way to tackle this would be to rebase onto the sui-move branch of the the move repo, to get mostly up to date with sui, then manually copy all our changes into the appropriate place in the sui tree.

@brson
Copy link
Collaborator Author

brson commented Dec 11, 2023

Well, good news is that rebasing onto the sui-move branch of the move repo is not too hard. Here is a first try:

https://github.com/brson/move/tree/solana-sui-base

The tests run, but not all tests pass.

@brson
Copy link
Collaborator Author

brson commented Dec 11, 2023

The IR test failures appear to mostly be because the compiler is emitting IR function definitions in a different order than on the main branch.

@brson
Copy link
Collaborator Author

brson commented Dec 12, 2023

The sui compiler had a curious change that caused move-build to seemingly not work with bytecode dependencies. Applying this patch made those tests pass again:

+++ b/language/move-compiler/src/command_line/compiler.rs
@@ -598,6 +598,8 @@ fn generate_interface_files_for_deps(
     let interface_files_paths =
         generate_interface_files(deps, interface_files_dir_opt, module_to_named_address, true)?;
     deps.extend(interface_files_paths);
+    // Remove bytecode files
+    deps.retain(|p| !p.path.as_str().ends_with(MOVE_COMPILED_EXTENSION));
     Ok(())
 }

@jcivlin
Copy link
Collaborator

jcivlin commented Dec 12, 2023

The sui compiler had a curious change that caused move-build to seemingly not work with bytecode dependencies. Applying this patch made those tests pass again:

+++ b/language/move-compiler/src/command_line/compiler.rs
@@ -598,6 +598,8 @@ fn generate_interface_files_for_deps(
     let interface_files_paths =
         generate_interface_files(deps, interface_files_dir_opt, module_to_named_address, true)?;
     deps.extend(interface_files_paths);
+    // Remove bytecode files
+    deps.retain(|p| !p.path.as_str().ends_with(MOVE_COMPILED_EXTENSION));
     Ok(())
 }

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397.
Screenshot 2023-12-11 at 10 12 16 PM

@brson
Copy link
Collaborator Author

brson commented Dec 14, 2023

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397.

Doesn't seem to be related. I applied the patch here but didn't see any change in how move-build responded to bytecode dependencies.

@brson
Copy link
Collaborator Author

brson commented Dec 14, 2023

Ok, this branch that is rebased onto the move/sui-move branch appears to pass all the tests it passed previously:

https://github.com/brson/move/tree/solana-sui-base

I have filed several new issues revealed during the rebase:

Next step is to attempt to transplant all our changes on to the sui repo's copy of move, which has a completely different layout than the move repo.

@jcivlin
Copy link
Collaborator

jcivlin commented Dec 15, 2023

Can it be bc we are processing modules in a reverse order. I found this to be problematic for DI building and restoring the order returned by move compiler, see #397.

Doesn't seem to be related. I applied the patch here but didn't see any change in how move-build responded to bytecode dependencies.

The patch controls the order of modules in the same file, so if you observe it in compilation of multiple files it may not help.

@brson
Copy link
Collaborator Author

brson commented Feb 23, 2024

Update:

My previous attempt to rebase attempt failed, where I was trying to rebase onto the latest (but old) sui branch of move, then apply diffs to the sui codebase. The sui codebase has changed so much that the diffs were just unappliable.

I started a new attempt that just moves our files into the sui codebase one at a times, and it has been more successful.

My branch is here: https://github.com/brson/sui/tree/solana

Most of the tests pass now. The major outstanding problem is that sui removed from move-cli all support for alternative architectures, so none of our patches can be applied directly. I have started re-introducing multi-architecture support to move-cli, and once that works almost all tests should pass again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants