contract: add read-only Soroban contract invocation#5946
Conversation
marwen-abid
force-pushed
the
feat/EN-51-contract-invoke-read
branch
from
282926c to
81f529e
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81f529e125
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if network == "" { | ||
| return nil, invalidArgsf("New: network passphrase is required") | ||
| } | ||
| addr, err := xdr.ScAddressFromStrkey(contractID) |
There was a problem hiding this comment.
When New is called with a syntactically valid G... or M... strkey, ScAddressFromStrkey succeeds even though this client is meant to target a contract. That invalid address is then copied into InvokeContractArgs.ContractAddress, so the constructor accepts the bad target and the first Invoke fails later during RPC simulation rather than being rejected up front. Validate that contractID is specifically a C... contract strkey before storing it here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Chat is right here, we'd want to only allow C...
There was a problem hiding this comment.
Pull request overview
This PR introduces a new contract package that provides a higher-level, read-only API for invoking Soroban smart contracts via build → simulate → decode, plus a set of safe, non-panicking xdr ScVal builder helpers for constructing Soroban arguments.
Changes:
- Added
contract.Clientandcontract.AssembledTransactionto construct and simulate SorobanInvokeHostFunctiontransactions and expose simulation outputs. - Added typed error classification (
contract.Error+ErrorKind+ sentinels) and auth/result helpers for interpreting simulation outcomes. - Added
xdrScv*builder helpers (includingScAddressFromStrkey) with validation and extensive round-trip tests.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| xdr/scval_builders.go | Adds validated, non-panicking Scv* helpers and ScAddressFromStrkey for building Soroban argument ScVals. |
| xdr/scval_builders_test.go | Unit tests for address decoding, symbol validation, 128-bit range checks, and deterministic map encoding. |
| xdr/scval_builders_roundtrip_test.go | Ensures Scv* builders round-trip through XDR marshal/unmarshal with stable type discriminants. |
| contract/result.go | Adds AssembledTransaction.Result() for retrieving simulated return values for read calls. |
| contract/errors.go | Introduces contract.Error/ErrorKind classification and errors.Is sentinels. |
| contract/errors_test.go | Tests error formatting, unwrap behavior, and classifier semantics. |
| contract/client.go | Adds contract.Client with functional options and Invoke() to build+simulate calls. |
| contract/client_test.go | Tests client construction, invocation behavior, options, and error classification. |
| contract/auth.go | Adds helpers to classify read calls and interpret simulation auth requirements. |
| contract/auth_test.go | Tests auth helper behavior including enforcing resimulation detection and signer extraction. |
| contract/assembled_transaction.go | Implements build/simulate lifecycle wrapper and simulation decoding/fee handling. |
| contract/assembled_transaction_test.go | Tests simulation folding, error paths, resource fee behavior, and Result() branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return false | ||
| } | ||
| for _, entry := range a.AuthEntries { | ||
| if entry.Credentials.Type == xdr.SorobanCredentialsTypeSorobanCredentialsAddress && |
There was a problem hiding this comment.
This will need to handle SorobanCredentialsTypeSorobanCredentialsAddressV2 after the protocol-next branch merges; it might make sense to retarget that branch so we have P27 support from the get-go
| if network == "" { | ||
| return nil, invalidArgsf("New: network passphrase is required") | ||
| } | ||
| addr, err := xdr.ScAddressFromStrkey(contractID) |
There was a problem hiding this comment.
Chat is right here, we'd want to only allow C...
| // ScvMap builds an SCV_MAP from a Go map with symbol keys. Entries are | ||
| // emitted in lexicographic order so callers get deterministic, canonical | ||
| // encoding regardless of map iteration order. | ||
| func ScvMap(kv map[string]ScVal) (ScVal, error) { |
There was a problem hiding this comment.
Love this but just fyi map keys don't have to be symbols, that's just how structs are expressed in soroban
There was a problem hiding this comment.
it's definitely a "simplification". But I figured it hits most use-cases we care about and we can just achieve the required ordering using sort.Strings vs the whole ScVal comparator. If it's too opinionated, happy to switch
| maxU128 = new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 128), big.NewInt(1)) // 2^128 - 1 | ||
| ) | ||
|
|
||
| func i128Parts(v *big.Int) (Int128Parts, error) { |
There was a problem hiding this comment.
This exists in the amount package, I'm pretty sure - better to not reinvent the wheel (many soldiers have fallen trying to get this right).
There was a problem hiding this comment.
The amount package only goes the other direction from what I can tell: String128 / String128Raw convert Int128Parts -> string. There is no big.Int -> Int128Parts/UInt128Parts constructor in amount.
I'm planning on using this builder later as the amounts will be represented as big.Int. If you think we should represent the amounts in another format, maybe I can take this builder out for now.
I've seen us use string in other areas but I'm not a big fan of that.
// Same five lines for XLM, USDC, or a pure SAC.
tok, _ := asset.New(xdr.MustNewNativeAsset(),
asset.WithRPC(rpc), asset.WithNetwork(network.TestNetworkPassphrase),
asset.WithSource(kp.Address()), asset.WithSigner(contract.KeypairSigner(kp)))
at, _ := tok.Transfer(ctx, kp.Address(), recipient, big.NewInt(10_000_000))
sent, _ := at.SignAndSend(ctx, contract.KeypairSigner(kp))
_, _ = sent.Wait(ctx)
| bumped := math.Ceil(float64(minResourceFee) * multiplier) | ||
| if bumped > math.MaxInt64 { | ||
| return math.MaxInt64 | ||
| } | ||
| return int64(bumped) |
There was a problem hiding this comment.
Could leverage min here (I've found claude and friends to be really bad at using modern Go features):
| bumped := math.Ceil(float64(minResourceFee) * multiplier) | |
| if bumped > math.MaxInt64 { | |
| return math.MaxInt64 | |
| } | |
| return int64(bumped) | |
| return min(int64(math.Ceil(float64(minResourceFee) * multiplier), math.MaxInt64) |
There was a problem hiding this comment.
I may be missing something here but min(int64(x), math.MaxInt64) seems like a no-op since every int64 is ≤ MaxInt64
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b05a137731
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| method, args := extractInvocation(params.Op.HostFunction) | ||
|
|
||
| tx, err := buildTx(params.SourceAccount, params.Op, params.BaseFee, params.Memo, params.Preconditions) |
There was a problem hiding this comment.
When callers use the exported NewAssembledTransaction directly and leave AssembleParams.Preconditions at its zero value as documented optional, this call passes that zero value into txnbuild.NewTransaction; txnbuild.Preconditions.Validate rejects zero TimeBounds because they were not constructed with NewTimebounds, NewTimeout, or NewInfiniteTimeout. Client.Invoke sets infinite time bounds, so the tests cover only that path, but direct users of this constructor cannot assemble a transaction unless they know to supply preconditions despite the API marking them optional.
Useful? React with 👍 / 👎.
| // and preserves first-seen order. | ||
| // | ||
| // It returns nil before Simulate has run. | ||
| func (a *AssembledTransaction) NeedsNonInvokerSigningBy() []xdr.ScAddress { |
There was a problem hiding this comment.
Should we consider naming this NeedsNonSourceAccountSigningBy or something that doesn't use the term "invoker"? I know the JS SDK (and maybe others?) uses this term so keeping it as-is may also make sense.
| // not run, so their footprint and resource fees require signing the entry and | ||
| // re-simulating in enforcing mode. Plain Ed25519 account-address signers do not | ||
| // need this. | ||
| func (a *AssembledTransaction) RequiresEnforcingResimulation() bool { |
There was a problem hiding this comment.
This is an awesome helper. I don't remember the JS SDK including this, and if its still not we should consider adding it, including the code comments on simulate() that reference it.
Adds a new contract package providing a higher-level surface for
invoking Soroban contracts, in a read-only form: it carries a generic
contract call from build -> simulate -> decoded return value. There is
no signer and no submission yet.
What lands:
- contract.Client (New, Invoke): generic invocation against any Soroban
contract. Invoke builds an InvokeHostFunction op, simulates it, and
returns an AssembledTransaction ready to read. Configured with
functional options: WithDefaultSource/WithSource, WithBaseFee,
WithResourceFeeMultiplier, WithMemo, WithTimeBounds. Read-only calls
need no funded source — they simulate against a synthetic null account.
- contract.AssembledTransaction (Simulate): models the Soroban
transaction lifecycle as an explicit, ctx-driven type. Simulate folds
the simulated SorobanData footprint, auth entries, and verbatim
minResourceFee back into a rebuilt transaction. A RestorePreamble is
detected and reported as not-yet-supported rather than silently
mishandled.
- Read helpers: Result() returns the simulated ScVal for view calls;
IsReadCall, NeedsNonInvokerSigningBy, and RequiresEnforcingResimulation
classify the simulation outcome.
- A typed error tree (Error{Kind}, ErrorKind, errors.Is sentinels) for
the lifecycle stages exercised here (invalid args, source-account,
simulation, not-yet-simulated).
- xdr SCVal builders (Scv*) and ScAddressFromStrkey: one-liner,
non-panicking constructors for host-function arguments that validate
symbols, 128-bit ranges, and strkey payloads up front.
Arguments are raw []xdr.ScVal for now. Native-value argument marshaling,
single-signer writes (signer + sign/send/poll), and auto-restore are not
included and will follow in later PRs.
marwen-abid
force-pushed
the
feat/EN-51-contract-invoke-read
branch
from
b05a137 to
9463cf1
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9463cf165a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de31f2920a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
marwen-abid
force-pushed
the
feat/EN-51-contract-invoke-read
branch
from
de31f29 to
3ea3683
Compare
- NewAssembledTransaction: default a zero-value Preconditions.TimeBounds to NewInfiniteTimeout (matching Invoke) so the documented-optional field builds without a factory-constructed TimeBounds. - validateSourceAddr: use keypair.ParseAddress, which length-checks the decoded payload, instead of the length-blind strkey.IsValidEd25519PublicKey. - auth: note that RequiresEnforcingResimulation / NeedsNonSourceAccountSigningBy scope to Address/AddressV2; delegated credentials land with multi-party signing. - Rename leftover NeedsNonInvokerSigningBy test names/comments.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cecae89bb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e8b33d156
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| InvokeContract: &xdr.InvokeContractArgs{ | ||
| ContractAddress: c.contractAddr, | ||
| FunctionName: *methodSym.Sym, | ||
| Args: args, |
There was a problem hiding this comment.
When a caller reuses or mutates the args slice after Invoke returns and then calls at.Simulate() again, the simulation request is encoded from the existing Built envelope while the rebuild uses a.op, whose InvokeContract.Args still aliases the caller's mutated slice. That can leave Simulation/ReturnValue describing the old call but Built containing different arguments, so copy the slice before assigning it to the operation.
Useful? React with 👍 / 👎.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
CHANGELOG.mdwithin the component folder structure. For example, if I changed horizon, then I updated (services/horizon/CHANGELOG.md. I add a new line item describing the change and reference to this PR. If I don't update a CHANGELOG, I acknowledge this PR's change may not be mentioned in future release notes.semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Adds a new
contractpackage providing a higher-level surface for invoking Soroban contracts. This PR is intentionally scoped to the read-only path: it carries a generic contract call frombuild → simulate → decoded return value, with no signer and no submission.Adds:
contract.Client(New,Invoke) — generic invocation against any contract.Invokebuilds anInvokeHostFunctionop, simulates it, and returns anAssembledTransactionready to read. Configured with functional options (WithDefaultSource/WithSource,WithBaseFee,WithResourceFeeMultiplier,WithMemo,WithTimeBounds). Read-only calls need no funded source — they simulate against a synthetic null account.contract.AssembledTransaction(Simulate) — models the Soroban transaction lifecycle as an explicit,ctx-driven type.Simulatefolds the simulatedSorobanDatafootprint, auth entries, and (verbatim, un-padded)minResourceFeeback into a rebuilt transaction.Result()returns the simulatedScValfor view calls;IsReadCall,NeedsNonInvokerSigningBy, andRequiresEnforcingResimulationclassify the simulation outcome.Error{Kind}witherrors.Issentinels (ErrInvalidArgs,ErrSourceAccountFailed,ErrSimulationFailed,ErrNotYetSimulated).xdrSCVal builders —Scv*one-liner, non-panicking constructors plusScAddressFromStrkey, validating symbols, 128-bit ranges, and strkey payloads up front.Why
Ultimately, we're building towards making it easier to send payments using Go. This is the first step.
It is deliberately landed read-only first so the core
xdr → client → simulate → decodepath can be reviewed against a real consumer and tests before the write path is built on top.Known limitations
Scoped to the read path; the following are planned for follow-up PRs:
Result()on a write call returnsErrNotYetSimulated.[]xdr.ScVal; marshaling native Go values into arguments is not here yet.RestorePreamblefrom simulation is surfaced as aKindSimulationFailederror rather than transparently restored + re-simulated.G…) strkeys; muxed/custom-sequenced sources require building withtxnbuilddirectly.