-
Notifications
You must be signed in to change notification settings - Fork 501
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
txnbuild: add max limit to change trust #1269
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Just some small changes.
txnbuild/change_trust.go
Outdated
type ChangeTrust struct { | ||
Line Asset | ||
Limit string | ||
SourceAccount Account | ||
} | ||
|
||
// MaxTrustlineLimit represents the maximum value that can be passed as trutline Limit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be passed as trutline Limit
-> can be set as a trustline limit.
txnbuild/change_trust.go
Outdated
"github.com/stellar/go/amount" | ||
"github.com/stellar/go/support/errors" | ||
"github.com/stellar/go/xdr" | ||
) | ||
|
||
// ChangeTrust represents the Stellar change trust operation. See | ||
// https://www.stellar.org/developers/guides/concepts/list-of-operations.html | ||
// https://www.stellar.org/developers/guides/concepts/list-of-operations.html. | ||
// If Limit is omitted, it defaults to txnbuild.MaxTrustlineLimit; the max int64 value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// If Limit is omitted, it defaults to txnbuild.MaxTrustlineLimit; the max int64 value.
-> // If Limit is omitted, it defaults to txnbuild.MaxTrustlineLimit.
type ChangeTrust struct { | ||
Line Asset | ||
Limit string | ||
SourceAccount Account | ||
} | ||
|
||
// MaxTrustlineLimit represents the maximum value that can be passed as trutline Limit | ||
var MaxTrustlineLimit = amount.StringFromInt64(math.MaxInt64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this be a const
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope you will get an error because the value is returned from a method
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestChangeTrustMaxLimit(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you confirm by looking e.g. in Stellar lab that the limit was set as expected in the XDR? If so this is fine - just include the link to the lab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep.. will add link
This PR adds the maximum trustline limit to a change trust operation when no limit is set.
Closes #1265