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: support Vector input for Scripts, improve overall Vector input support #1046

Merged
merged 42 commits into from
Jul 13, 2023

Conversation

camsjams
Copy link
Contributor

@camsjams camsjams commented Jun 6, 2023

Fixes #1015

This is a total rewrite of how Vectors work, removing the previous version I implemented and packing Vector data alongside its pointer, to be dynamically appended at the last step of the Abi Coder, see concatWithVectorData.

  • Improves support for Vectors across the board
    • For context, existing support (currently available) for Vector is - Vector, Struct in Vector, Enum in Vector, Numbers/Strings in Vector
    • This PR adds more advanced usage, matching what is supported in the Rust SDK, such as Vector in a struct, Vector in a Vector, Vector in an array, Vector in an enum etc
  • Adds support for Vectors in Scripts
    • Vector input to Scripts was never tested or explicitly supported by TS-SDK
    • Luckily the change was to set correct offset instead of the default 0
  • Adds partial support for Vectors in Predicates, continuation in Improve Predicate Vector input support  #1083
    • Vector input to Predicates was also never tested or explicitly supported by TS-SDK
    • Likewise we just had to set correct offset instead of the default 0
    • Currently this is only tested with Predicate Coins
  • Adds new tests to validate Sway types in Vectors across all three Program modes

Also see:

@camsjams camsjams requested a review from a team June 6, 2023 01:31
@camsjams camsjams self-assigned this Jun 6, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2023

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
84.07% (-0.11% 🔻)
4623/5499
🟡 Branches
62.9% (-0.05% 🔻)
651/1035
🟡 Functions
68.99% (-0.16% 🔻)
732/1061
🟢 Lines
84.25% (-0.12% 🔻)
4462/5296
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / index.ts
100% 100% 100% 100%
🟢
... / index.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / abi-coder.ts
83.67% (-0.16% 🔻)
72.09% 90%
83.16% (-0.18% 🔻)
🟢
... / utilities.ts
85.19% (-14.81% 🔻)
100% (+12.5% 🔼)
91.67% (-8.33% 🔻)
84% (-16% 🔻)
🔴
... / script-invocation-scope.ts
15.38% (-1.28% 🔻)
0% 0%
15.38% (-1.28% 🔻)
🔴
... / script-request.ts
26.67% (-0.61% 🔻)
0%
9.09% (-0.91% 🔻)
26.67% (-0.61% 🔻)

Test suite run success

1014 tests passing in 182 suites.

Report generated by 🧪jest coverage report action from 941c23d

@camsjams camsjams marked this pull request as draft June 6, 2023 01:51
@camsjams camsjams changed the title chore: add test for vector cases feat: support Vector input for Scripts and Predicates Jun 6, 2023
@camsjams camsjams changed the title feat: support Vector input for Scripts and Predicates feat: support Vector input for Scripts and Predicates, improve overall Vector input support Jun 6, 2023
@camsjams
Copy link
Contributor Author

It appears that with this more complex case, the vector input argument is not being encoded as expected.

@Torres-ssf Ok I've updated the implementation to better handle data like that, and I've added some more tests to account for the changes, including two Sway applications and some unit tests.

Dhaiwat10
Dhaiwat10 previously approved these changes Jul 12, 2023
Copy link
Member

@Dhaiwat10 Dhaiwat10 left a comment

Choose a reason for hiding this comment

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

🪄

Copy link
Contributor

@nedsalk nedsalk left a comment

Choose a reason for hiding this comment

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

A nit on the return statements of AbiCoder and a request for change/explanation on the use of double tildes.

packages/abi-coder/src/abi-coder.ts Outdated Show resolved Hide resolved
packages/abi-coder/src/utilities.ts Outdated Show resolved Hide resolved
packages/abi-coder/src/utilities.ts Outdated Show resolved Hide resolved
danielbate
danielbate previously approved these changes Jul 13, 2023
Copy link
Contributor

@danielbate danielbate left a comment

Choose a reason for hiding this comment

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

LGTM, really appreciate the more complex test cases being used also

nedsalk
nedsalk previously approved these changes Jul 13, 2023
Torres-ssf
Torres-ssf previously approved these changes Jul 13, 2023
@camsjams camsjams dismissed stale reviews from Torres-ssf, nedsalk, and danielbate via 941c23d July 13, 2023 17:18
@camsjams camsjams enabled auto-merge (squash) July 13, 2023 17:18
@nedsalk nedsalk self-requested a review July 13, 2023 17:27
@camsjams camsjams merged commit b1c240a into master Jul 13, 2023
7 checks passed
@camsjams camsjams deleted the cm/issue-1015 branch July 13, 2023 17:28
@camsjams camsjams changed the title feat: support Vector input for Scripts and Predicates, improve overall Vector input support feat: support Vector input for Scripts, improve overall Vector input support Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Passing Vec to script causes random values inside script
6 participants