-
Notifications
You must be signed in to change notification settings - Fork 502
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
Add integration test for extra signers using tx preconditions #4320
Add integration test for extra signers using tx preconditions #4320
Conversation
services/horizon/internal/integration/transaction_preconditions_test.go
Outdated
Show resolved
Hide resolved
// has also signed this transaction. | ||
txParams.Preconditions.ExtraSigners = []string{addtlAccounts[0].GetAccountID()} | ||
_, err = itest.SubmitMultiSigTransaction([]*keypair.Full{master, addtlSigners[0]}, txParams) | ||
tt.NoError(err) |
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.
We should also make sure that retrieving the tx by ID shows the extra signers in the response, otherwise we aren't really testing Horizon at all besides txsub
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.
great suggestion, I added round trip asserts on the precond values in each of the test cases, thanks!
…bmitted tx per pr feedback
I just saw that the verify-range test is failing, I tried to rerun it |
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, LGTM!
new integration test case for using the
Precondtions.ExtraSigners
parameter with a standard AccountID as the additional signer.