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

feat: sign forc-deploy transactions #2629

Merged
merged 10 commits into from
Sep 1, 2022
Merged

feat: sign forc-deploy transactions #2629

merged 10 commits into from
Sep 1, 2022

Conversation

kayagokalp
Copy link
Member

@kayagokalp kayagokalp commented Aug 26, 2022

About this PR

closes #2623.

Blocked until FuelLabs/fuels-rs#540 is merged and released. While testing I pointed to a local instance of the SDK, most of the diff is actually coming from the lock file.

TODO

  • Accept transaction parameters
  • Point to released fuels-rs (Blocked until the next release of fuels-rs)

@mitchmindtree
Copy link
Contributor

Just adding a note (previously discussed on slack with @kayagokalp):

I think it might be best if we first wait to update fuel-core, fuels-rs and their dependencies that we use throughout the Sway repo before this lands. We want to avoid the massive lock file additions this PR would otherwise cause, which would require anyone building the repo to build both entire version sets and likely slow down CI quite a bit.

#2622 tracks the fuel-core update blockers in particular.

@mitchmindtree
Copy link
Contributor

Ahh it looks like #2482 is doing the heavy-work of updating to the latest fuel-core and fuels-rs.

@kayagokalp kayagokalp requested review from a team and mitchmindtree August 31, 2022 17:57
@kayagokalp kayagokalp marked this pull request as ready for review August 31, 2022 17:58
@kayagokalp kayagokalp enabled auto-merge (squash) August 31, 2022 17:59
@kayagokalp kayagokalp changed the title refactor: forc-deploy requires wallet address and accepts signature feat: sign forc-deploy transactions Aug 31, 2022
mitchmindtree
mitchmindtree previously approved these changes Sep 1, 2022
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

🚀

Looking great! Just a couple conflicts to resolve.

sezna
sezna previously approved these changes Sep 1, 2022
@kayagokalp kayagokalp dismissed stale reviews from sezna and mitchmindtree via 208248e September 1, 2022 19:39
@kayagokalp kayagokalp requested a review from a team September 1, 2022 19:57
@kayagokalp kayagokalp merged commit 7cde107 into master Sep 1, 2022
@kayagokalp kayagokalp deleted the kayagokalp/2623 branch September 1, 2022 21:57
mitchmindtree added a commit that referenced this pull request Sep 2, 2022
…formatting fixes (#2698)

* refactor: forc-deploy requires wallet address and accepts signature (#2629)

* Add the `CopyTypes` trait to `DeclarationId` (#2682)

Co-authored-by: Toby Hutton <[email protected]>

* fix: Unformatted comment spans add extra newline (#2692)

* newline handler checks for existing newlines before inserting new ones

* stability test added

* newline-comment handler interaction test added

* review suggestion

* feat: add basic comment context formatting (#2697)

* feat: add comment context formatting

* test: enhance newline-comment handler interaction test

* Apply suggestions from code review

Co-authored-by: mitchmindtree <[email protected]>

Co-authored-by: mitchmindtree <[email protected]>

* Update `examples/` for recent swayfmt-v2 patches

Co-authored-by: Kaya Gökalp <[email protected]>
Co-authored-by: Emily Herbert <[email protected]>
Co-authored-by: Toby Hutton <[email protected]>
eureka-cpu added a commit that referenced this pull request Sep 2, 2022
* update plugin and swayfmt toml, remove old formatter

* update config to formatter for consistency

* wip fix lsp formatting

* more merge conflicts

* update dependencies to 22.1

* remove files that made it back in from merge

* comment out function in LSP that uses formatter

* format examples and remove debug printlns

* Merge #2669 (swayfmt replacement PR) with `master` including newline formatting fixes (#2698)

* refactor: forc-deploy requires wallet address and accepts signature (#2629)

* Add the `CopyTypes` trait to `DeclarationId` (#2682)

Co-authored-by: Toby Hutton <[email protected]>

* fix: Unformatted comment spans add extra newline (#2692)

* newline handler checks for existing newlines before inserting new ones

* stability test added

* newline-comment handler interaction test added

* review suggestion

* feat: add basic comment context formatting (#2697)

* feat: add comment context formatting

* test: enhance newline-comment handler interaction test

* Apply suggestions from code review

Co-authored-by: mitchmindtree <[email protected]>

Co-authored-by: mitchmindtree <[email protected]>

* Update `examples/` for recent swayfmt-v2 patches

Co-authored-by: Kaya Gökalp <[email protected]>
Co-authored-by: Emily Herbert <[email protected]>
Co-authored-by: Toby Hutton <[email protected]>

* change name to swayfmt, kashira

* add swayfmt file

* sort toml dependencies

* fix excess newlines in format_context

* test on examples

Co-authored-by: mitchmindtree <[email protected]>
Co-authored-by: Kaya Gökalp <[email protected]>
Co-authored-by: Emily Herbert <[email protected]>
Co-authored-by: Toby Hutton <[email protected]>
Co-authored-by: Mohammad Fawaz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request forc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor forc-deploy so that it returns an transaction ID and asks for a signature
3 participants