-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
refactor(x/auth): Revert signature of TxSignBatchExec #18602
Conversation
Fix suite_test.go
WalkthroughThe overall change involves modifying the Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- tests/integration/auth/client/cli/suite_test.go (4 hunks)
- x/auth/CHANGELOG.md (1 hunks)
- x/auth/client/testutil/helpers.go (1 hunks)
Additional comments: 7
tests/integration/auth/client/cli/suite_test.go (4)
159-171: The calls to
authtestutil.TxSignBatchExec
correctly pass a single filename string instead of a slice of strings, aligning with the updated function signature.212-224: The introduction of
batchSignArgs
and its use in constructing arguments forclitestutil.ExecTestCLICmd
is correctly implemented and aligns with the summary.798-803: The call to
authtestutil.TxSignBatchExec
correctly passes a single filename string instead of a slice of strings, aligning with the updated function signature.808-813: The call to
authtestutil.TxSignBatchExec
correctly passes a single filename string instead of a slice of strings, aligning with the updated function signature.x/auth/CHANGELOG.md (1)
- 36-36: The changelog entry correctly documents the reversion of the
TxSignBatchExec
signature and provides a link to the associated pull request.x/auth/client/testutil/helpers.go (2)
66-72: The change from a slice of filenames to a single filename in
TxSignBatchExec
is correctly implemented in the hunk. Ensure that all usages of this function in the codebase are updated to pass a single filename string.63-73: The summary of the pull request indicates a reversion of the function signature, but the hunk shows a change from accepting a slice of filenames to a single filename. Please clarify if the summary or the hunk is incorrect.
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/e2e/auth/suite.go (6 hunks)
Additional comments: 6
tests/e2e/auth/suite.go (6)
226-263: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.288-293: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.1171-1176: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.1181-1186: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.1244-1249: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.1254-1259: The changes to
TxSignBatchExec
function calls to use a single string argument for the file name are consistent with the summary and correctly implemented.
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.
lgtm!, ty!
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/e2e/auth/suite.go (6 hunks)
Additional comments: 4
tests/e2e/auth/suite.go (4)
228-230: The error message assertion should be updated to reflect the new error message format if it has changed due to the reversion of
TxSignBatchExec
function's signature.1171-1177: The
--multisig
flag is used correctly to enable multi-signature mode for the transaction signing process. Ensure that theTxSignBatchExec
function supports this flag as expected.1244-1250: The use of
--offline
flag along with--account-number
and--sequence
is appropriate for offline transaction signing. Verify that theTxSignBatchExec
function handles these flags correctly after the reversion.288-294: The assertion for the sequence number after signing the batch (
s.Require().Equal(seq+1, seq1)
) is correct, but ensure that the sequence number is being incremented as expected in the actual implementation.
Description
This PR partially reverts the changes introduced in #18564:
cli/suite_test.go:TxSignBatchExec
to its previous signaturecli/suite_test.go:TestCLISignBatchTotalFees
to useclitestutil.ExecTestCLICmd
instead ofTxSignBatchExec
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
in the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
Refactor
Tests
Documentation
Bug Fixes