txnbuild: Add txnbuild helper for sending a payment to a smart contract #5642

tamirms · 2025-03-25T18:41:22Z

PR Checklist

PR Structure

This PR has reasonably narrow scope (if not, break it down into smaller PRs).
This PR avoids mixing refactoring changes with feature changes (split into two PRs
otherwise).
This PR's title starts with name of package that is most changed in the PR, ex.
services/friendbot, or all or doc if the changes are broad or impact many
packages.

Thoroughness

This PR adds tests for the most critical parts of the new functionality or fixes.
I've updated any docs (developer docs, .md
files, etc... affected by this change). Take a look in the docs folder for a given service,
like this one.

Release planning

I've reviewed the changes in this PR and if I consider them worthwhile for being mentioned on release notes then I have updated the relevant CHANGELOG.md within 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.
I've decided if this PR requires a new major/minor version according to
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

Close #5626

Update txnbuild to support constructing SAC payments to contracts with default fees so that simulateTransaction calls to rpc are not necessary.

Also fixes a bug in txnbuild where the max fee in the transaction envelope did not include the additional soroban resource fee.

Why

see #5626

Known limitations

[N/A]

Shaptic · 2025-03-26T20:11:51Z

txnbuild/invoke_host_function.go

+		return InvokeHostFunction{}, err
+	}
+
+	transferArgs := xdr.ScVec{


Seems like the Go SDK could certainly use a nativeToScVal equivalent, but that'd be a separate PR+refactor of course.

txnbuild/invoke_host_function.go

txnbuild/restore_footprint.go

Shaptic · 2025-03-26T20:18:35Z

txnbuild/transaction.go

@@ -915,8 +914,16 @@ func NewTransaction(params TransactionParams) (*Transaction, error) {
 		if err != nil {
 			return nil, errors.Wrap(err, fmt.Sprintf("failed to build operation %T", sorobanOp))
 		}
+		if envelope.V1.Tx.Ext.SorobanData != nil {


I think either this should be done higher up or the uint32 check should be done after this, otherwise we miss the "proper" validation on L853 in this case.

@Shaptic I just pushed a fix for this issue. can you take a look?

txnbuild/invoke_host_function.go

Copilot

Pull Request Overview

This PR adds txnbuild helpers for sending a payment to a smart contract and for restoring an asset balance via a new restore footprint operation. It also updates fee handling and adjusts integration tests to use the new helper signatures while removing legacy fee parameters.

Introduces NewAssetBalanceRestoration and NewPaymentToContract functions with configurable Soroban fee values.
Updates integration tests to use the new Preflight functions without separate minFee values.
Refactors code across txnbuild and integration tests to align with these helper changes.

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated no comments.

File	Description
txnbuild/restore_footprint.go	Adds a helper for constructing asset balance restoration operations.
txnbuild/invoke_host_function.go	Adds a helper for constructing payment-to-contract operations with updated fee handling.
Integration tests (various files)	Adjust test calls to use the new helper function signatures by removing extra fee parameters.
Horizon integration tests	Update fee-bump and Preflight function calls to support the new txnbuild APIs.

Comments suppressed due to low confidence (2)

services/horizon/internal/integration/integration.go:816

Double-check that all callers of PreflightHostFunctions have been updated to handle the new signature returning a single value, as the removal of minFee could cause runtime issues if any outdated expectation remains.

return function

services/horizon/internal/integration/invokehostfunction_test.go:130

[nitpick] Consider defining a named constant for 'HostFunctionTypeHostFunctionTypeUploadContractWasm' to improve code clarity and reduce potential typos in repeated usage.

assert.Equal(t, invokeHostFunctionOpJson.Function, "HostFunctionTypeHostFunctionTypeUploadContractWasm")

sreuland · 2025-04-08T20:26:12Z

services/horizon/internal/test/integration/integration.go

@@ -214,6 +214,10 @@ func NewTest(t *testing.T, config Config) *Test {
 		}

 		i.WaitForHorizonIngest()
+
+		if !config.SkipCoreContainerCreation && !i.config.QuickExpiration {


minor, could consolidate all condition checks to single point inside upgradeLimits() after existing check on protocolversion.

sreuland · 2025-04-08T20:35:46Z

txnbuild/invoke_host_function.go

+	Amount string
+	// Asset is the asset being transferred
+	Asset Asset
+	// SourceAccount is the source account of the payment


Suggested change

// SourceAccount is the source account of the payment

// SourceAccount is the source account of the payment, it must be a Stellar account in strkey encoded`VersionByteAccountID` format, i.e. a 'G' account.

sreuland · 2025-04-08T20:40:22Z

txnbuild/invoke_host_function.go

+	// Destination is the contract recipient of the payment
+	Destination string
+	// Amount is the amount being transferred
+	Amount string


could Amount be strongly typed to a float64 instead, more intuitive for callers?

sreuland · 2025-04-08T20:48:06Z

txnbuild/invoke_host_function.go

+	}
+
+	resources := params.Fees
+	if resources.ResourceFee == 0 {


could there ever be legit use case for setting fee to 0? If remotely plausible, it may be worthwhile to define PaymentToContractParams.Fees as a pointer *SorobanFees so can check for explicit absence before applying defaults.

sreuland · 2025-04-08T21:05:46Z

txnbuild/invoke_host_function.go

+		Auth: []xdr.SorobanAuthorizationEntry{
+			{
+				Credentials: xdr.SorobanCredentials{
+					Type: xdr.SorobanCredentialsTypeSorobanCredentialsSourceAccount,


Based on this hardcode, should source account authentication be stated as limitation in docs forNewPaymentToContract usage?

Can auth aspect be configurable via something like PaymentToContractParams.Auth SorobanAuth with SorobanAuth as a new struct for defining auth type and sub-fields such as when type is SOROBAN_CREDENTIALS_ADDRESS? Maybe this latter address type auth requires client to assemble tx and invoke preflight manually, meaning they should not use NewPaymentToContract in that case?

sreuland · 2025-04-08T21:20:45Z

txnbuild/transaction.go

+	hi, lo := bits.Mul64(uint64(params.BaseFee), uint64(len(inner.operations)+1))
+	if hi > 0 || lo > math.MaxInt64 {
+		return nil, errors.Errorf("base fee %d results in an overflow of max fee", params.BaseFee)
+	}
+	totalFee := lo + sorobanFee
+	if totalFee < lo || totalFee > math.MaxInt64 {
+		return nil, fmt.Errorf("soroban fee %v results in an overflow of max fee", sorobanFee)
+	}
+	tx.maxFee = int64(totalFee)


minor, lines 1006-1014 looks like duplication of same code on lines 916-925, might be worth refactor to internal, reusable fn?

tamirms added 8 commits March 25, 2025 18:40

Add txnbuild helper for sending a payment to a smart contract

e803381

fix build error

078840c

fix TestClaimableBalanceCreationOperationsAndEffects

463b211

fix integration tests

3de368d

fix more integration tests

f3724f7

Merge branch 'master' into sc-payment

ea4e32d

fix TestStateVerifier

774872f

Add more tests

22804b8

tamirms marked this pull request as ready for review March 26, 2025 20:02

tamirms requested review from a team and marcelosalloum March 26, 2025 20:05

Shaptic reviewed Mar 26, 2025

View reviewed changes

tamirms commented Mar 26, 2025

View reviewed changes

txnbuild/invoke_host_function.go Show resolved Hide resolved

leighmcculloch requested a review from Copilot March 27, 2025 06:04

Copilot AI reviewed Mar 27, 2025

View reviewed changes

overcat mentioned this pull request Mar 27, 2025

fix: fix the issue where sending assets using Transaction.append_payment_to_contract_op fails when the sender's account is the same as the asset issuer's account. StellarCN/py-stellar-base#1029

Merged

fix fee logic

76a951b

tamirms force-pushed the sc-payment branch from 055a2be to 76a951b Compare March 28, 2025 12:45

fix maxFee for feebump

1503aff

sreuland reviewed Apr 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

txnbuild: Add txnbuild helper for sending a payment to a smart contract #5642

txnbuild: Add txnbuild helper for sending a payment to a smart contract #5642

tamirms commented Mar 25, 2025 •

edited

Loading

Shaptic Mar 26, 2025

Shaptic Mar 26, 2025

tamirms Mar 28, 2025

Copilot AI left a comment

sreuland Apr 8, 2025

sreuland Apr 8, 2025

sreuland Apr 8, 2025

sreuland Apr 8, 2025

sreuland Apr 8, 2025

sreuland Apr 8, 2025

	// SourceAccount is the source account of the payment
	// SourceAccount is the source account of the payment, it must be a Stellar account in strkey encoded`VersionByteAccountID` format, i.e. a 'G' account.

txnbuild: Add txnbuild helper for sending a payment to a smart contract #5642

Are you sure you want to change the base?

txnbuild: Add txnbuild helper for sending a payment to a smart contract #5642

Conversation

tamirms commented Mar 25, 2025 • edited Loading

PR Structure

Thoroughness

Release planning

What

Why

Known limitations

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tamirms commented Mar 25, 2025 •

edited

Loading