feat: API for uploading a sway project #19
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
sdankel
changed the title
| } | ||
|
|
||
| /// Installs the given version of forc at the specific root path using cargo-binstall. | ||
| pub fn install_forc_at_path(forc_version: &str, forc_path: &Path) -> Result<(), UploadError> { |
There was a problem hiding this comment.
Instead of using the CLI; i'd reccomend using library/SDK functions to perform uploading operations.
They would be much lighter and ephemeral as one would not need to have the binary installed.
^ could be a future PR; just something to keep in mind
There was a problem hiding this comment.
Are you saying to use forc-pkg to build the sway project rather than using the forc CLI? Or are you suggesting using a library to install forc? If you know of libraries that can do this I'd be interested, I looked around but couldn't find one.
There was a problem hiding this comment.
Yea i was saying we shouldn't rely on the user having forc-binaries installed; implying requiring external deps - outside this project.
Note: Could be a future PR though as an improvement.
There was a problem hiding this comment.
I might be a little lost here but isn't this a server that we are running? What users are we referring to exactly?
My understanding was that: we run this service, once a user want's to actually publish a project, forc upload (or whatever the cli command will be) will make an API call into this service uploading the project.
Then the server will get the project, and extract the bytecode identifier from it (bytecode identifier in this context would basically be an id generated from the existing bincode that "generalizes" the project, meaning that the effect of different initial configurable values etc are removed to get an id that describes only the "core" of the contract)
There was a problem hiding this comment.
@kayagokalp has it right. This isn't expecting a user to have forc binary installed, it's installing it on the server (running in docker in the cloud) and using it to build the sway project that it received via the upload API.
We wouldn't be able to use forc-pkg since we have to support any version of forc.
|
Nice! Happy to approve once updated and you're happy for another review. |
sdankel
requested review from
JoshuaBatty,
zees-dev,
mchristopher and
alfiedotwtf
|
|
||
| // Clean up the temporary directory. | ||
| fs::remove_dir_all(upload_dir).map_err(|_| ApiError::Upload(UploadError::SaveFile))?; | ||
| tmp_dir |
There was a problem hiding this comment.
whole point of tempfile or tempdir is not requiring explicit cleanup since the Drop trait for this should do this when the variable goes out of scope (I think)
There was a problem hiding this comment.
The docs say to call tmpdir.close()
| uses: taiki-e/install-action@cargo-binstall | ||
| - name: Install forc for tests | ||
| run: | | ||
| cargo binstall --no-confirm --root tests/fixtures/forc-0.65.0 [email protected] |
There was a problem hiding this comment.
Should the version string be dynamic so it doesn't have to be touched every update?
Just a question, happy for this to be merged though.
There was a problem hiding this comment.
There's an argument for it but usually we don't want tests to start failing because of a release in another repo.
| /// A mock implementation of the PinataClient trait for testing. | ||
| #[cfg(test)] | ||
| pub struct MockPinataClient; | ||
|
|
||
| #[cfg(test)] | ||
| impl PinataClient for MockPinataClient { | ||
| async fn new() -> Result<Self, UploadError> { | ||
| Ok(MockPinataClient) | ||
| } | ||
|
|
||
| async fn upload_file_to_ipfs(&self, _path: &Path) -> Result<String, UploadError> { | ||
| Ok("ABC123".to_string()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Really not important but these two can be moved to a mod test if that sounds good to you (to follow a more conventional style), totally ok if we leave them as is though!
|
|
||
| /// Handles the project upload process by unpacking the tarball, compiling the project, copying the | ||
| /// necessary files to a new directory, and storing the source code tarball and ABI file in IPFS. | ||
| /// Returns a NewUpload struct with the necessary information to store in the database. |
There was a problem hiding this comment.
| /// Returns a NewUpload struct with the necessary information to store in the database. | |
| /// Returns a `NewUpload` struct with the necessary information to store in the database. |
| } | ||
|
|
||
| /// Installs the given version of forc at the specific root path using cargo-binstall. | ||
| pub fn install_forc_at_path(forc_version: &str, forc_path: &Path) -> Result<(), UploadError> { |
There was a problem hiding this comment.
I might be a little lost here but isn't this a server that we are running? What users are we referring to exactly?
My understanding was that: we run this service, once a user want's to actually publish a project, forc upload (or whatever the cli command will be) will make an API call into this service uploading the project.
Then the server will get the project, and extract the bytecode identifier from it (bytecode identifier in this context would basically be an id generated from the existing bincode that "generalizes" the project, meaning that the effect of different initial configurable values etc are removed to get an id that describes only the "core" of the contract)
Closes #18
This PR adds a new API,
upload_project, that accepts a .tgz file containing a sway project and does the following:Notes: