Merge `sway-lib-core` and `sway-lib-std` into this repository #1052

mitchmindtree · 2022-03-28T07:35:18Z

OK! After wading through a long list of blockers I think we might be ready to try and land this :)

As discussed in #830, this PR merges the sway core and std libraries into the sway repository.

This doesn't yet address #330, but is one of the final steps towards doing so.

Examples and tests have been updated to depend on core and std via path. This should be useful for ensuring that any changes to either core or std are accounted for in the E2E tests, while also ensuring that any changes to sway-core or forc are accounted for in core and std under CI.

I've added two more dedicated CI jobs for building sway-lib-core and sway-lib-std so that it should be clear by glancing at a PR if either of those are the cause of CI failure.

At the very least I'll wait to get ⛔ or ✔️ from @adlerjohn and @nfurfaro as both have pending PRs at sway-lib-std, but I've also requested reviews from most folk who touch each of these repos regularly in case anyone else has feedback too!

Closes #830

History-preserving merge

Rather than copying across the contents of the directories, I've attempted to preserve our history from the sway-lib-core and sway-lib-std repos by git mergeing both into this repository.

To do so, I used the following approach for both:

$ git remote add fuellabs/sway-lib-core git@github.com:FuelLabs/sway-lib-core.git
$ git fetch fuellabs/sway-lib-core
$ git merge fuellabs/sway-lib-core/master --allow-unrelated-histories
# Resolve conflicts

Conflicts were only related to README.md, .gitignore and ci.yml, but otherwise went smoothly.

How does `std` depend on `core`?

One issue I noticed when beginning to merge in std is that to ensure std and core are synchronised under each commit, std needs to depend on core via path as there is of course no way to depend on a library via a commit that doesn't exist yet.

I thought std's use of path for the core dependency might cause issues for projects that would depend on std remotely via git. However it turns out that forc can handle path in dependencies of dependencies without issue as long as the dependency's path declaration is relative and points to a location within the same git repository.

Intuitively I think this behaviour makes sense and I'm yet to think of a good case against it. That said, I think it should be clearly documented and enforced by forc with friendlier errors in a follow-up PR.

Existing `sway-lib-std` PRs

cc @adlerjohn @nfurfaro I see that there are a few PRs remaining at sway-lib-std. Would you like to land those first? Or are you happy to rebase that work onto this repo after this lands? It would be nice if we could avoid too many merge conflicts, but I can manage it fine if you're worried about rebasing the existing work.

Follow-up

Migrate the ./sway-lib-std/tests into the E2E stdlib directory and get them all working. Migrate the ./sway-lib-std/tests into the E2E stdlib directory and get them all working #1058
Migrate the sway-lib-core and sway-lib-std issues and PRs to this repo. Move issues from sway-lib-core and sway-lib-std into sway issue tracker #1059

* Add basic build CI workflow. * Remove installation and use of fuel-core.

Added "modulo" function so the % operator works Very simple implementation to all the other functions just wrapping the mod opcode

* Add Result type. * Fix syntax.

* Update ops.sw * Weird spacing thing

* Add ecr lib * Add todo comment * Add recover_pubkey and refactor ecr * Fixup * Update comments * Add comments to ec-recover asm block * Modify asm * Clean up asm * Fix B512 field assignment * Fix ordering of lib deps * Remove ec_recover, to be added in separate PR * Clean up comments

* Add todo comment * Add working ec_recover implementation

* Add token lib * Add todo comment * Rename token_id to asset_id * Rename token_id to asset_id * Rename token_id to asset_id * Fix register names in asm block * Fixup * Refactor ugly while loop * Fixup * Remove ecr from branch * Fix review issues * Modify type definition to silence warning re losing precision via type-cast * Fix asm block registers * Add only the mint function * Ad burn to token lib * "fix toml file"

* Moved eq into its own trait (Eq) Moved eq into it's own trait (Eq) and then added all the implementations for u64, u32, u16, u8, and b256 * Forgot to delete u16's old Eq implementation * Move le to a test implementation * Fixed ge, le, and neq to not use eq and instead use inline assembly * Update ops.sw * Formatting changes

* Add force_transfer function to token lib * Add import of ContractID type

* Impl Ord for Address * Fix todo comment * Fix toml formatting * Add comment to use meq * Impl Eq for Address * Fix formatting * Remove todo

* Add more methods to Result. * Don't use catch all. * Use catch up, but fix typo. * Fix return type. * Compile workarounds. * fmt * Remove todo comments. * fmt * Make Result public.

* Feat:Add Option type & is_some/is_none methods * Doc:Improve module-level docs * Fix:Add generic type T to match

* Simplify the B512 (and therefore ECR) code. Instead of 2 b256 fields in a struct we can use a 2 element array instead. I found that both the assumptions about memory layout and all the copying was producing brittle code, and simplifying the ASM blocks as much as possible is a good thing. * Do a `forc fmt`.

* Add force_transfer function to token lib * Add import of ContractID type * Add transfer_to_output function to token lib * Add imports and temp remove constants * Add else clause to if branch * Add better comment * Add final else clause to if branch * Add semicolons to if-else * style: add a comment and remove trailing semicolon from if statement in return position * fix: make some comments internal * fix: re-enable constants * fix: change OUTPUT_LENGTH_LOCATION from 48 to 56 * fix: clean up assembly and incorporate PR review feedback * docs: fix comments to reflect change to OUTPUT_LENGTH_LOCATION * fix: remove redundent local variable assignment * refactor: split asm blocks to avoid a second usage of the xos opcode * style: forc fmt * fix:change offset to 40 for getting amount

* Add module level docstring description * Rearrange lib.sw order * Use result type and add Sender type * Improve function documentation * refactor: improve clarity by removing the ! operator * refactor: pass in constants to asm blocks * fix: remove redundent `dep address;` from lib.sw * fix: switch consts to enum * refactor: revert to using constants for now but added TODO

* config:update gitignore to ignore all tagket dirs * feat:add working demo of test harness * fix:add modules * Add new demo_script_test project * build:update dep versions in manifest files * fix:update code in mod and main * fix:change author email * fixup * fix:use local path to ttest current stdlib * test:remove dummy test * cleanup:remove dummy tests in prep for merge * style:cargo fmt

* test:add token-ops tests * test:update structure * test:add more test * test:adds more comments to failing tests * test:remove failing tests from PR * style:cargo fmt * stle:forc fmt * style:fix formatting I missed * test:cleanup and remove logs

sezna

This is pretty high-touch, but if i'm not mistaken, the only really new lines of code are the ci job you added? In which case, LGTM, and I love the visibility we get into breaking changes with those separated CI jobs.

sezna · 2022-03-28T09:23:04Z

Do want to wait on John and Nick's thoughts on their PRs though.

mitchmindtree · 2022-03-28T10:18:01Z

This is pretty high-touch, but if i'm not mistaken, the only really new lines of code are the ci job you added?

Yeah exactly, just the CI jobs and updating the examples and tests to point to std and core via path.

.github/workflows/ci.yml

examples/counter/Forc.lock

adlerjohn · 2022-03-28T13:24:27Z

I don't care about my open PR(s) for the stdlib, they need master anyways.

nfurfaro · 2022-03-28T14:38:09Z

@mitchmindtree I'm fine to rebase my WIP PRs onto the sway repo after this lands.

* Add tx module. * Add functions. * Clean up. * Add skeleton test artifact project. * Use hard-coded constants since compiler can't handle consts. * Fill in skeleton. * Make untested get typed script data non-pub. * Add output functions. * Comment out unreachable. * Improve name. * Add remaining functions. * Fix name. * Register tests to harness. * Add tests. * Fix script start offset test. * Fix script start offset get. * Fix test with todo. * Add non-working test for input coin owner. * Return wallet. * Fix test.

adlerjohn · 2022-03-28T15:27:17Z

If possible though, could you also include:

* fixing transfer_to_output * Update transfer_to_output * Improving a comment * fmt

* Add changes from prior work * Remove import of ops * Cleanup and format * Rename functions to match opcode naming * Re-order lib.sw * Fix types to use ContractId * Refactor and cleanup * Reorder lib.sw * Clean up comments and types * FIx lib deps ordering * cleanup * congig:update gitignore * Commit stashed changes with get_coin_owner WIP * Fix:cleanup merge conflict resolution mistake * Clean up a bit and use Result. * General cleanup. * Improve comment. * Fix a bunch of type errors. * Add missed assert. * Fix order to make compile. * Refactor. * fix * Fix build errors. * Update offsets for 255 max inputs. * Remove warnings. * Improve error name. * Fix some asm. * Fix more asm * space * Fix comment * Fix offsets. * Refactor to use new tx module. * test: add auth test project * test: register auth tests * test: add tests to mod.rs * test: add auth testing abi & contract * test: add auth caller contract & script * test: fix enum variant name * test: fix import of result * test: refactor types to workaround MissingData error from SDK * test: fix number of args passed * test: ignore script test * style: forc fmt * style: cargo fmt * test: work on script test * Remove redundant assert. * Clean up tests, not working yet. * Fix test for some reason. * Fix test. Co-authored-by: Nick Furfaro <nfurfaro33@gmail.com>

* test: update path to assert * Delete build.sh Added by accident.

…hmindtree/merge-core-std

adlerjohn · 2022-03-28T23:10:08Z

assert was moved, see FuelLabs/sway-lib-std#97. ~~Not sure how this wasn't caught for the stdlib!~~ Ah it's the examples that are broken, my bad.

mitchmindtree · 2022-03-28T23:11:52Z

assert was moved, see FuelLabs/sway-lib-std#97. Not sure how this wasn't caught for the stdlib!

No worries, fixing these tests now :) These sorts of issues will be harder let through once it's all merged heh.

adlerjohn

Green checkmark means go. Would like @otrho to take a look too.

mitchmindtree · 2022-03-29T01:29:29Z

🥂

adlerjohn and others added 30 commits December 28, 2021 13:31

Initial commit

14b67de

Initial commit

5b98072

initial commit

c415f89

intial commit

d6021fa

update manifest to use git core dependency

5e7f3c4

add README

77a735d

Add README

54190d9

Fix link to forc (#1)

c658d72

Fix link to forc (#1)

fc96209

Add basic build CI workflow (#2)

2620261

* Add basic build CI workflow. * Remove installation and use of fuel-core.

Implement Modulo (#3)

0e7235b

Added "modulo" function so the % operator works Very simple implementation to all the other functions just wrapping the mod opcode

.gitattributes to highlight .sw syntax as rust (#3)

3c9b9c5

.gitattributes to highlight .sw syntax as rust (#5)

5f29bb0

Add Result type (#6)

1617341

* Add Result type. * Fix syntax.

Add basic build CI workflow. (#8)

267ae8a

Add in rsh and lsh operations (#6)

87138fb

* Update ops.sw * Weird spacing thing

Adding the missing ec_recoverfunctionality (#18)

15bed83

* Add todo comment * Add working ec_recover implementation

Format manifest file. (#10)

45c54ab

Adding the force_transfer function to the token lib. (#27)

193114a

* Add force_transfer function to token lib * Add import of ContractID type

Implement the Eq trait for Address (#25)

888f0ab

* Impl Ord for Address * Fix todo comment * Fix toml formatting * Add comment to use meq * Impl Eq for Address * Fix formatting * Remove todo

Add more methods to Result. (#16)

68fcc90

* Add more methods to Result. * Don't use catch all. * Use catch up, but fix typo. * Fix return type. * Compile workarounds. * fmt * Remove todo comments. * fmt * Make Result public.

Add Option type & is_some/is_none methods (#31)

07feb84

* Feat:Add Option type & is_some/is_none methods * Doc:Improve module-level docs * Fix:Add generic type T to match

mitchmindtree requested review from emilyaherbert and mohammadfawaz March 28, 2022 07:35

mitchmindtree self-assigned this Mar 28, 2022

sezna previously approved these changes Mar 28, 2022

View reviewed changes

adlerjohn reviewed Mar 28, 2022

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

examples/counter/Forc.lock Show resolved Hide resolved

mohammadfawaz and others added 5 commits March 28, 2022 16:09

Update transfer to output (#94)

ae8def5

* fixing transfer_to_output * Update transfer_to_output * Improving a comment * fmt

Update path to assert (#97)

0d77393

* test: update path to assert * Delete build.sh Added by accident.

test: remove redundant test (#99)

dc03647

Merge remote-tracking branch 'fuellabs/sway-lib-std/master' into mitc…

b0fc736

…hmindtree/merge-core-std

mitchmindtree dismissed sezna’s stale review via b0fc736 March 28, 2022 22:54

mitchmindtree added 3 commits March 29, 2022 10:57

Update examples and tests for chain::assert -> assert::assert

de7b45a

Merge branch 'master' into mitchmindtree/merge-core-std

22070b5

Update newly merged test for chain::assert -> assert::assert

255a0d5

adlerjohn approved these changes Mar 29, 2022

View reviewed changes

otrho approved these changes Mar 29, 2022

View reviewed changes

mitchmindtree merged commit fbdce29 into master Mar 29, 2022

mitchmindtree deleted the mitchmindtree/merge-core-std branch March 29, 2022 01:30

This was referenced Mar 29, 2022

Migrate the ./sway-lib-std/tests into the E2E stdlib directory and get them all working #1058

Closed

Commit working Forc.lock files to ensure that core is pinned to a working state #1080

Closed

sezna mentioned this pull request Mar 29, 2022

bump to v0.9.1 #1088

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Merge `sway-lib-core` and `sway-lib-std` into this repository #1052

Merge `sway-lib-core` and `sway-lib-std` into this repository #1052

Uh oh!

mitchmindtree commented Mar 28, 2022 •

edited

Loading

Uh oh!

sezna left a comment

Uh oh!

sezna commented Mar 28, 2022

Uh oh!

mitchmindtree commented Mar 28, 2022

Uh oh!

Uh oh!

Uh oh!

adlerjohn commented Mar 28, 2022

Uh oh!

nfurfaro commented Mar 28, 2022

Uh oh!

adlerjohn commented Mar 28, 2022 •

edited by mitchmindtree

Loading

Uh oh!

adlerjohn commented Mar 28, 2022 •

edited

Loading

Uh oh!

mitchmindtree commented Mar 28, 2022

Uh oh!

adlerjohn left a comment

Uh oh!

mitchmindtree commented Mar 29, 2022

Uh oh!

Uh oh!

Merge sway-lib-core and sway-lib-std into this repository #1052

Merge sway-lib-core and sway-lib-std into this repository #1052

Uh oh!

Conversation

mitchmindtree commented Mar 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

History-preserving merge

How does std depend on core?

Existing sway-lib-std PRs

Follow-up

Uh oh!

sezna left a comment

Choose a reason for hiding this comment

Uh oh!

sezna commented Mar 28, 2022

Uh oh!

mitchmindtree commented Mar 28, 2022

Uh oh!

Uh oh!

Uh oh!

adlerjohn commented Mar 28, 2022

Uh oh!

nfurfaro commented Mar 28, 2022

Uh oh!

adlerjohn commented Mar 28, 2022 • edited by mitchmindtree Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adlerjohn commented Mar 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mitchmindtree commented Mar 28, 2022

Uh oh!

adlerjohn left a comment

Choose a reason for hiding this comment

Uh oh!

mitchmindtree commented Mar 29, 2022

Uh oh!

Uh oh!

Merge `sway-lib-core` and `sway-lib-std` into this repository #1052

Merge `sway-lib-core` and `sway-lib-std` into this repository #1052

mitchmindtree commented Mar 28, 2022 •

edited

Loading

How does `std` depend on `core`?

Existing `sway-lib-std` PRs

adlerjohn commented Mar 28, 2022 •

edited by mitchmindtree

Loading

adlerjohn commented Mar 28, 2022 •

edited

Loading