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

Support constructors in contract creation via Operation.createCustomContract #770

Merged
merged 6 commits into from
Sep 12, 2024

Conversation

Shaptic
Copy link
Contributor

@Shaptic Shaptic commented Aug 23, 2024

Upgrade the createCustomContract abstraction to accept a new, optional parameter:

class Operation {
  // ...
  createCustomContract(
    address: Address,
    wasmHash: Buffer | Uint8Array,
+   constructorArgs?: xdr.ScVal[],
    salt?: Buffer | Uint8Array,
...

and upgrade the internals to use the CreateContractV2 host function.

This closes #768.

Copy link

github-actions bot commented Aug 23, 2024

Size Change: +2.31 kB (+0.07%)

Total Size: 3.26 MB

Filename Size Change
dist/stellar-base.js 2.39 MB +1.9 kB (+0.08%)
dist/stellar-base.min.js 867 kB +408 B (+0.05%)

compressed-size-action

@chadoh
Copy link
Contributor

chadoh commented Aug 26, 2024

Why ctorArgs instead of just args or constructorArgs?

@Shaptic
Copy link
Contributor Author

Shaptic commented Aug 27, 2024

@chadoh will you accept "laziness" as an answer? 😆 fixed!

@Shaptic Shaptic marked this pull request as ready for review August 27, 2024 20:24
@Shaptic Shaptic force-pushed the add-invocation-abstraction branch from 4e30bde to b368f81 Compare August 27, 2024 20:25
@Shaptic Shaptic requested review from sreuland and a team August 27, 2024 20:29
@Shaptic Shaptic changed the title Add abstraction for constructor-based contract creation Support constructors in contract creation via Operation.createCustomContract Sep 12, 2024
@Shaptic Shaptic force-pushed the add-invocation-abstraction branch from 0bea18a to 5e2b7d4 Compare September 12, 2024 17:48
@@ -91,7 +91,12 @@ export const AuthClawbackEnabledFlag = 1 << 3;
* * `{@link Operation.setTrustLineFlags}`
* * `{@link Operation.liquidityPoolDeposit}`
* * `{@link Operation.liquidityPoolWithdraw}`
* * `{@link Operation.invokeHostFunction}`
* * `{@link Operation.invokeHostFunction}`, which has the following additional
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// note: using a string here doesn't work because once the operation
// is encoded/decoded it will be a Buffer internally and it'd be a
// mild pain in the ass to check equivalence
nativeToScVal(Buffer.from('admin name')),
Copy link
Contributor

Choose a reason for hiding this comment

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

understood, but, it may be worthwhile to demonstrate plain string usage here as it represents typical client code scenario(and test is used as a reference by devs and followed verbatim), and then add the clunky buffer conversion down later for just test assertion purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fair enough :) way to advocate for the user 😎 done in 8450ac2!

Copy link
Contributor

@sreuland sreuland left a comment

Choose a reason for hiding this comment

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

really clean integration and test coverage!

@Shaptic Shaptic merged commit f645fc2 into protocol-22 Sep 12, 2024
7 checks passed
@Shaptic Shaptic deleted the add-invocation-abstraction branch September 12, 2024 19:17
Shaptic added a commit that referenced this pull request Sep 17, 2024
* Add TypeScript definitions
* Make code fmt action separate from tests
Shaptic added a commit that referenced this pull request Oct 31, 2024
* Upgrade generated XDR definitions to Protocol 22 (#769)
* Upgrade XDR to the latest tips of `curr` and `next` branches (#774)
* Support contracts w/ constructors via `createCustomContract` (#770)
* Prepare v13.0.0-beta.1 (Protocol 22) for release. (#779)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants