Migrate forc-client from the sway repo
#143
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Closes #3190 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
## Description closes #4155. unblocks #4156. Since we are using the build profile derived from build flags of forc for nodes in the dependency graph, contract dependencies can end-up getting compiled with tests (we are fine if we compile with `forc build` but this issue surfaces once we use `forc build --tests`). This is not something we want because of two reasons: 1. In terms of `forc-pkg`: We shouldn’t allow this as inserted contract id and deployed contract id should be the same for the feature to work all the time. (This is the case if you run forc build today but if you run forc build --tests the inserted contract ids are essentially wrong, if you have some tests in those packages) 2. In terms of `forc-test`: The contract deployment’s are done without the tests so their contract ids are calculated without the tests, since forc-pkg inserts the contract id calculated with tests included. The actual contract id in the interpreter instance and injected contract id is different. This means calls using the following abi declaration is failing: `let abi = abi(MyContractABIinContractDependency, contract_dependency::CONTRACT_ID)` . Basically we cannot use contract id namespace injection feature with the unit tests. `forc-test` related problems are not apparent yet since multi contract calls are not enabled (they will be with #4156). ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
## Description Reference to external dependency struct now looks like: `"type": "struct abi_with_tuples::Person",` `"type": "enum std::option::Option",` Struct reference in root module still looks like: `"type": "struct MyStruct",` Struct reference in sub modules looks like: `"type": "struct dep_1::MyStruct1",` ` forc build` will not generate json ABI with call path by default to do so we have to use the flag `--json-abi-with-callpaths`, one example is: ```cargo run --bin forc build --json-abi-with-callpaths --path `pwd`/test/src/e2e_vm_tests/test_programs/should_pass/test_contracts/abi_with_same_name_types/``` This will generate `test/src/e2e_vm_tests/test_programs/should_pass/test_contracts/abi_with_same_name_types/out/debug/abi_with_same_name_types-abi.json` with call paths for structs and enums. This is related to: FuelLabs/fuel-specs#450 FuelLabs/fuels-rs#791 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
## Description closes #3571. closes #4162. This PR adds the ability of calling multiple contracts from sway unit tests if they are added as `[contract-dependencies]`. This is limited with contracts currently but I will be having a follow-up which builds upon this to introduce this support to scripts as well. As these contracts are already declared under `[contract-dependencies]` their contract ids are injected into their namespace by `forc-pkg`. A bug related to this step is fixed in #4159. <img width="787" alt="image" src="https://user-images.githubusercontent.com/20915464/224345002-92dc2bcb-823d-4971-9041-31111cf85e77.png"> ### Follow-ups - #4161 - ~#4162~ ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Kaya Gokalp <[email protected]>
## Description Closes #3681 I meant to tackle #3077 first, but tackling #3681 first cleans up a lot of the code needed to make #3077 easier. This PR is mostly cleanup: - Completely removes usage of `ConfigTimeConstant` everywhere (in forc-pkg, sway-core, sway-type) - **(BREAKING)** Completely removes support for `[constants]` table in the manifest, which was using the `ConfigTimeConstant` struct. This change is breaking for app devs ([open issue](FuelLabs/sway-applications#375)) and from a [quick search](https://github.com/FuelLabs/sway-applications/search?l=TOML&q=%5Bconstants%5D) on our sway-applications repo, there's a number of manifests still using the old version of configuration time constants. Consequentially, the above change also allowed us to cut down on other constructs like `ConstInjectMap` within forc-pkg, since the only thing we need to inject now are const CONTRACT_IDs - this cuts down quite a bit of now-redundant code. - As a result of the removal of `ConfigTimeConstant`, a few tests using that feature were also deleted. - Also updated the `configurable` example to be known as `configurable_constants` instead of `config_time_constant`. **This PR does not:** - Fix the manual creation of `CONTRACT_ID` const. To create the const, we still undergo manual parsing and type-checking and finally inserting into a `SymbolMap` manually. This will be handled in a separate PR to tackle #3077 Other than removal of a lot of code to do with CTC, the main change here that allows contract dependencies to still work here is instead of relying on the `ConstInjectMap` which we used to inject both constants and contract dependencies, there is now only a single contract ID value that we require (this is abstracted as `ContractIdConst`) to inject into the contract for tests. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
Moves following `Cargo.toml` fields to workspace `Cargo.toml` so we don't have to keep repeating ourselves: - edition - authors - homepage - license - repository
## Description Closes #4117 Allow multiple salt declarations in `forc deploy`, with parsing and validation involved. The gist of it is that it collects all the salt inputs by the user as a `Vec<String>`, parses each input as a `Salt` and store them in a `ContractSaltMap` matching contract->salt, and later compares them against the contract dependencies to make sure that duplicate salts do not exist. This throws an error if: 1) multiple salts are present (if the same salt is declared but also found within a contract dependency), 2) if a salt is invalid, 3) if the salt input is invalid (there's a difference - the salt input is the string `<CONTRACT>:<SALT>`, while the salt above is `fuel_tx::Salt`, which has its own validation. 4) duplicate salts for the same contract were provided in the CLI. Tests consist of dummy folders (not entire full package initialized by forc) with only manifests to keep it the bare minimum required for these tests. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers. --------- Co-authored-by: Mohammad Fawaz <[email protected]>
## Description Adds `forc contract-id` and `forc predicate-id` commands for contract id and predicate root detection. closes #3444.
Also ran `cargo update` because this is a breaking release. Pending: - [x] FuelLabs/sway#4391 - [x] FuelLabs/sway#4390 - [x] FuelLabs/sway#4385
## Description
graph.visualize now can include urls that are clickable in generated svg
files.
To set this up we can call forc with the argument
`--dca-graph-url-format`.
For vscode we would use: `--dca-graph-url-format
'vscode://file/{path}:{line}:{col}'`
This commit also changes forc `--dca-graph` to receive a path.
We still can output to std by using `--dca-graph ''`.
Also adds edge labels.
Also changes entry node shape to be different, a double octagon instead
of an ellipse.
## Checklist
- [ ] I have linked to any relevant issues.
- [ ] I have commented my code, particularly in hard-to-understand
areas.
- [ ] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
…tFile::new` (#4412) ## Description Fixes missing validation step in the `PackageManifestFile::new()`. closes #4411.
## Description This PR introduces `StorageHandle` to the `core` as an **experimental** feature. `StorageHandle` is a descriptor of a location in storage containing two fields: 1. A key pointing to a storage slot `s`. 2. An offset pointing to a particular word in `s` or in its subsequent slots. This offset is new and is not described in the RFC. I will rectify that shortly. The standard library introduces helper methods `read`, `try_read`, and `write` to `StorageHandle` that allow reading from and writing to the location pointed to by the handle. `try_read` returns an `Option` wrapping the data while `read` automatically internally unwraps the data and could panic. The summary of this change is as follows: * New struct `core::experimental::storage::StorageHandle` * New storage library `std::experimental::storage::*` that should eventually replace `std::storage::*`. This new library mirrors the old one to the extent possible and introduces the helper methods for `StorageHandle`. Other data structures such as `StorageVec` and `StorageBytes` will be introduced in a separate PR. * Add an experimental flag `--experimental-storage` to `forc` that enables all the required code in the compiler to support `StorageHandle` as described in the FuelLabs/sway-rfcs#23. * Type checking phases assumes the existence of `StorageHandle` and monomorphizes it as needed. * Storage accesses now always return `StorageHandle` and storage reassignment are no longer relevant. * IR gen handles storage accesses by "filling" the key and the offset in an aggregate representing the `StorageHandle`. The key and the offset are statically determined based on the index of the storage variable in the `storage` block and the field accessed, if applicable. * Storage initializers are now handled based on the new model * The new approach packs struct fields by default but does not pack across storage variables. This offers the most amount of flexibility. This is a deviation from the RFC which I will update shortly. * To implement `StorageMap` and other dynamic storage data structures, we now write `impl StorageHandle<StorageMap<K, V>>` instead of `impl StorageMap<K, V>` directly. Also, note that the `get` method now returns a `StorageHandle` instead of the actual data. To get the actual data, `read()` or `try_read()` should be called on the resulting handle. This is needed for multiple reasons including proper support for nested dynamic storage types. Rust also does the same, in a way (where `get` actually returns ` &` which happens to coerce to the real data in certain places). * I added various tests but they're not comprehensive. Some tests on my list: * Extensive tests for storage maps in structs (which now work!) * Extensive tests for storage accesses with various types and struct layouts I still need to figure out how to do nested dynamic storage types with this approach. The stuff I have in `map_in_map_access` in `test_projects/experimental_storage/src/main.sw` is just an attempt but not ergonomic at all of course. ## Checklist - [ ] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers.
closes #4374 @kayagokalp rightfully mentioned in the issue that the package descriptor is already contained within the `BuiltPackage` struct, which means we do not need this representation anymore: > This is a nice abstractions but it made me think a little and I think returning Vec of (PackageManifestFile, Arc<BuiltPackage>) might be unnecessary at the first place from built_pkg_with_manifest function. > > Every built package contains a package descriptor which contains the related package manifest so returning a tuple is kind of a data duplication. ## Description ## Checklist - [x] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
…` RFC (#4464) ## Description Implement FuelLabs/sway-rfcs#23 Closes FuelLabs/sway#3419 (technically via the RFC) Closes FuelLabs/sway#3202 Closes FuelLabs/sway#3043 Closes FuelLabs/sway#2639 Closes FuelLabs/sway#2465 Closes FuelLabs/sway#4304 This is a big one but mostly removes stuff. It's hard to review but the summary of changes below should be sufficient. Most of the changes here are things that I just had to do to make CI pass. - Removed the `--experimental-storage` flag and made its [behavior](FuelLabs/sway#4297) the default. Also removed everything that was required for the previous storage APIs in favour of the new ones. - Break down the `std::storage` into multiple submodules: - `storage_key` implements the API for `std::core::StorageKey` - `storage_api` implements the free functions `read` (previously `get`), `write` (previously `store`), and `clear`. - 4 more modules for the dynamic storage types which now use the new `StorageKey` API. - `#[storage(write)]` now allows reading from storage as well. This is needed because the way we pack structs in storage now requires that we sometimes read from storage first if we were to write a portion of a slot. - Removed the "storage only types" checks and the corresponding tests. - Removed the `__get_storage_key` intrinsic and the `get_storage_key` IR instruction and all corresponding tests. Also removed the `state_index` metadata as it is no longer required. - Added tests and example to showcase nested storage maps and nested storage vectors. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. Co-authored-by: Joshua Batty <[email protected]>
See here for context: FuelLabs/sway#4492 (comment) This follows a re-release of 0.37.1 as 0.37.3: https://github.com/FuelLabs/sway/releases/tag/v0.37.3 The outcome of all this is that ideally, users previously on 0.37.1 will now pull 0.37.3 next time they run `cargo update` and will not encounter any breakage. Those ready to update can point to 0.38.0 after this is published. Also runs `cargo update` in order to update deps to their latest patch release. --------- Co-authored-by: Joshua Batty <[email protected]>
## Description This change mainly adds checks to enforce the new module privacy rules and supporting changes for it. Changes include updating std and core to use public modules, updating the parser to allow the use of the `pub mod` syntax and adding an error type for private modules. This change is implemented behind a `--experimental-private-modules` experimental flag and not enabled by default. It implements part of FuelLabs/sway#4446, the `pub use` syntax is yet to be implemented. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers. Co-authored-by: Joshua Batty <[email protected]>
## Description Set experimental module privacy rules as the default and remove the experimental flag. Closes #4506 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
## Description Adds an initial benchmarking setup to CI, where a collection of libraries from https://github.com/FuelLabs/sway-libs are compiled, performance metrics generated and later pushed into a new repo: https://github.com/FuelLabs/sway-performance-data Closes FuelLabs/sway#318. ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
The PR fixes breakings changes from the `fuel-core 0.18.1` and `fuels-rs 0.41`. - The `__smo` is reworked and doesn't require an output index because we removed the `Output::Message` variant. - Replaced the old deploy API with a new one, `Contract::deploy` -> `Contract::load_from().deploy()`. - The signing of the transaction and predicate id calculation requires `ChainId` now. It breaks the API in some places. In tests, I used the default chain id, but from the `forc` perspective we need to add the ability to specify it. - `fuels-signers` was renamed into `fuels-accounts`. - We reworked `fuel_tx::Input` and now each variant of the enum has a named type. - Replaced all unsafe code from `fuel-crypto` with safe analog. - On the `fuel-tx`/`fuel-core` side now, there is a difference in whether the message contains data. If data is empty, it is `MessageCoin` that acts like a `Coin`(has the same rules during execution). If the data field is not empty, then it is a retryable message(or `MessageData`). Messages like this can't be used to pay a transaction fee, and if the execution fails, the message is not consumed(you can use it again in the next transactions). More about them you can read in FuelLabs/fuel-core#946. Also, the API changed for resources and not it is `Coins` again. Because of that, some tests now require messages with empty data to be able to spend them. - Removed redundant usage of `MessageId` and corresponding fields. Now the identifier of the message is a `Nonce`. - Removed `Output::Message`. Now you don't need to specify it in the outputs. Because of that `SMO` opcode doesn't require a message output index anymore. - We unified block height(in some places it was `u32` in some `u64`) by introducing the `BlockHeight` type. - The `nonce` in the `Message` is a `Bytes32` now instead of `u64`(affected `input_message_nonce` GTF). - Reworked the handling of the `Intrinsic::Smo`. Previously `fuel-vm` expected `smo(r1: receipt_and_message_ptr, r2: size_of_the_message, r3: output_index, r4: amount)` but we updated that to be `smo(r1: receipt_ptr, r2: message_ptr, r3: size_of_the_message, r4: amount)` - according to the specification. - Removed `input_message_msg_id` GTF. - Now tokens should be transferred to the contract first, and after you can transfer them via `SMO` or another transfer. So in some tests first we need to transfer money to the contract first. - The `fuels-rs` now generates a separate structure for encoder `{Contract_name}Encoder`. This PR is based on the FuelLabs/fuels-rs#950 and causes cycle dependencies. The `fuels-rs` should be released first and after we can apply it. --------- Co-authored-by: Sophie Dankel <[email protected]> Co-authored-by: iqdecay <[email protected]> Co-authored-by: Kaya Gökalp <[email protected]>
## Description Waiting for: - #4525. @Dhaiwat10 was waiting for a release with the linked PR merged. @IGI-111 I had the PR ready, leaving it up to you when to merge & cut the release. Made this a major bump as we had a breaking change merged (#4574). ## Checklist - [ ] I have linked to any relevant issues. - [ ] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [ ] I have added tests that prove my fix is effective or that my feature works. - [ ] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [ ] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [ ] I have requested a review from the relevant team or maintainers. Co-authored-by: IGI-111 <[email protected]>
## Description This PR bumps sdk to 0.43.0, and fuel-core to 0.18.2.
JoshuaBatty
force-pushed
the
josh/forc-client-migration
branch
from
8f48edf to
858d097
Compare
|
Thanks for the contribution! Before we can merge this, we need @mohammadfawaz @spiral-ladder @AlicanC to sign the Fuel Labs Contributor License Agreement. |
|
Thanks for the contribution! Before we can merge this, we need @mohammadfawaz @spiral-ladder @AlicanC @IGI-111 to sign the Fuel Labs Contributor License Agreement. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
.github/workflows/ci.yml
Outdated
| matrix: | ||
| include: | ||
| - args: --all-targets --all-features -- --test-threads 1 | ||
| - args: --all-targets --no-default-features -- --test-threads 1 |
There was a problem hiding this comment.
New test job missing timeout from original configuration
Low Severity
The new cargo-test job is missing a timeout-minutes setting. The original workflow had tests in cargo-verifications with a 30-minute timeout and comment "disallow any job that takes longer than 30 minutes". When tests were extracted to the separate cargo-test job, this timeout was not carried over. Without a timeout, tests could hang indefinitely (up to GitHub's 6-hour default), wasting CI resources and blocking the pipeline.
| .map_err(|e| format!("Invalid ABI path '{abi_path_str}': {e}"))?; | ||
|
|
||
| Ok((contract_id, abi_path)) | ||
| } |
There was a problem hiding this comment.
URL parsing broken in contract ABI argument parser
Medium Severity
The parse_contract_abi function uses split(':') to separate the contract ID from the ABI path. This breaks for URL-based ABI paths like https://example.com/abi.json because URLs contain colons after the scheme. When parsing 0x1234:https://example.com/abi.json, the split produces three parts (["0x1234", "https", "//example.com/abi.json"]) instead of two, causing try_into() to fail. The documentation explicitly shows URL support at line 259, but the implementation doesn't handle this case. Using splitn(2, ':') would correctly split only on the first colon.
|
|
||
| // Default to treating as file path | ||
| Ok(AbiSource::File(PathBuf::from(s))) | ||
| } |
There was a problem hiding this comment.
file:// URL not converted to proper file path
Medium Severity
The AbiSource::try_from implementation doesn't properly handle file:// URLs. When a user provides file:///path/to/abi.json, it parses as a URL but falls through the scheme check (only "http", "https", "ipfs" are handled). The entire URL string is then passed to PathBuf::from(), creating a path literal file:///path/to/abi.json instead of extracting /path/to/abi.json. When load_abi() tries to read this path, it will fail with "file not found". The test at line 529-530 only verifies the enum variant matches, not that the path is correct.
| #[clap(long)] | ||
| pub submit_only: bool, | ||
| /// Set the key to be used for signing. | ||
| pub signing_key: Option<SecretKey>, |
There was a problem hiding this comment.
Missing clap attribute makes signing_key a positional argument
Medium Severity
The signing_key field in both deploy.rs and run.rs is missing the #[clap(long)] attribute, causing it to be treated as a positional argument instead of a named option. In contrast, call.rs correctly defines it with #[clap(long, env = "SIGNING_KEY", help_heading = "ACCOUNT OPTIONS")]. This inconsistency means users must pass the signing key as a positional argument in deploy/run commands rather than using --signing-key, which is unexpected CLI behavior and inconsistent with the call command.
Additional Locations (1)
| let parts: Vec<&str> = s.trim().split(':').collect(); | ||
| let [contract_id_str, label] = parts | ||
| .try_into() | ||
| .map_err(|_| format!("Invalid label format: '{s}'. Expected format: contract_id:label"))?; |
There was a problem hiding this comment.
Label parsing broken when labels contain colons
Low Severity
The parse_label function uses split(':') which splits on all colons in the string, same pattern as parse_contract_abi. If a user provides a label containing a colon (e.g., --label 0x123:Block:123 or --label 0x123:Main:Contract), the split produces more than two parts, causing try_into::<[_; 2]>() to fail. Using splitn(2, ':') would correctly split only on the first colon, allowing colons in label values.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| result.push(c); | ||
| if c == '"' && previous_char != Some('\\') { | ||
| in_string = false; | ||
| } |
There was a problem hiding this comment.
Escaped backslash handling incorrect in JSON minifier
Low Severity
The minify_json function incorrectly determines whether a quote is escaped by only checking if the immediately preceding character is a backslash. This fails for escaped backslashes (\\) followed by a quote. For JSON like {"key":"test\\"}, the sequence \\ is an escaped backslash, so the following " should end the string. However, since previous_char is \, the code incorrectly treats the quote as escaped and continues the string, potentially corrupting the minified output.
3 tasks
JoshuaBatty
added a commit
to FuelLabs/sway-nightly-binaries
that referenced
this pull request
Feb 4, 2026
`forc-client` moved from `FuelLabs/sway` to `FuelLabs/forc` as of v0.71.0 (see FuelLabs/forc#143). This PR updates nightly builds to source it from the new location. Changes: - Add `forc-client` to the forc monorepo workflow - Handle multi-binary crates by extracting names from [[bin]] sections - Remove `forc-deploy`, `forc-run`, `forc-submit`, `forc-call` from sway workflow - Update README to document the new source Part of the tooling monorepo migration: FuelLabs/sway-rfcs#49
JoshuaBatty
added a commit
to FuelLabs/sway-nightly-binaries
that referenced
this pull request
Feb 4, 2026
`forc-client` moved from `FuelLabs/sway` to `FuelLabs/forc` as of v0.71.0 (see FuelLabs/forc#143). This PR updates nightly builds to source it from the new location. Changes: - Add `forc-client` to the forc monorepo workflow - Handle multi-binary crates by extracting names from [[bin]] sections - Remove `forc-deploy`, `forc-run`, `forc-submit`, `forc-call` from sway workflow - Update README to document the new source Part of the tooling monorepo migration: FuelLabs/sway-rfcs#49
JoshuaBatty
added a commit
to FuelLabs/sway-nightly-binaries
that referenced
this pull request
Feb 4, 2026
`forc-client` moved from `FuelLabs/sway` to `FuelLabs/forc` as of v0.71.0 (see FuelLabs/forc#143). This PR updates nightly builds to source it from the new location. Changes: - Add `forc-client` to the forc monorepo workflow - Handle multi-binary crates by extracting names from [[bin]] sections - Remove `forc-deploy`, `forc-run`, `forc-submit`, `forc-call` from sway workflow - Update README to document the new source Part of the tooling monorepo migration: FuelLabs/sway-rfcs#49
JoshuaBatty
added a commit
to FuelLabs/sway-nightly-binaries
that referenced
this pull request
Feb 4, 2026
`forc-client` moved from `FuelLabs/sway` to `FuelLabs/forc` as of v0.71.0 (see FuelLabs/forc#143). This PR updates nightly builds to source it from the new location. Changes: - Add `forc-client` to the forc monorepo workflow - Handle multi-binary crates by extracting names from [[bin]] sections - Remove `forc-deploy`, `forc-run`, `forc-submit`, `forc-call` from sway workflow - Update README to document the new source Part of the tooling monorepo migration: FuelLabs/sway-rfcs#49
JoshuaBatty
added a commit
to FuelLabs/sway-nightly-binaries
that referenced
this pull request
Feb 4, 2026
`forc-client` moved from `FuelLabs/sway` to `FuelLabs/forc` as of v0.71.0 (see FuelLabs/forc#143). This PR updates nightly builds to source it from the new location. Changes: - Add `forc-client` to the forc monorepo workflow - Handle multi-binary crates by extracting names from [[bin]] sections - Remove `forc-deploy`, `forc-run`, `forc-submit`, `forc-call` from sway workflow - Update README to document the new source Part of the tooling monorepo migration: FuelLabs/sway-rfcs#49
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Migrates
forc-clientfrom the sway repository into the forc monorepo, continuing the toolchain consolidation effort from sway-rfcs#49.Changes
New Workspace Member
forc-tracingandforc-wallet(within monorepo)sway-core,sway-types,sway-features,sway-utilsforc,forc-pkg,forc-debug,forc-tx,forc-utilNew Workspace Dependencies
sway-core,sway-features,sway-types,sway-utilsforc,forc-debug,forc-pkg,forc-txfuel-abi-types,fuel-core-client,fuel-core-storage,fuel-tx,fuel-vm,fuels,fuels-accounts,fuels-coreaws-config,aws-sdk-kmschrono,devault,either,k256,pretty_assertions,rexpect,toml_editRemoved Unused Dependencies
sway-ast,sway-parse(were in Cargo.toml but never imported)Binaries
forc-deployforc-runforc-submitforc-callPost-merge Tasks
forc-client-0.71.0forc-clientfrom this repoforc-clientforc-clientfrom hereforc-clientfrom sway repoRelated
forc-nodefrom theswayrepo #130forc-tracing&forc-cryptoover from theswayrepo. #119forc-walletrepository toforcmonorepo #1