deps: bump SDK to v0.47.1 #3459
Conversation
crodriguezvega
requested review from
colin-axner,
AdityaSripal,
charleenfei,
damiannolan and
chatton
as code owners
|
I am a bit stuck here... I bump interchaintest to the latest ( interchaintest was having the same issue, but in the latest commit that was fixed and it works now. But I am lost why we have this error in ibc-go... |
|
I think the problem is fixed! Thanks to @misko9 for the solution to the puzzle! |
|
looks like tests are still failing |
I know :) This is now a different problem. |
|
Looking a bit more into the failures, I attempted running the tests using a simd from main, but with interchain test pointing at sdk 0.47.1 and ran into the same account sequence mismatch error. Looking at the SDK changes I do suspect it has something to do with either CLI args (used by interchain test) or the |
|
After some more debugging with @charleenfei , we managed to figure it out. The problem was because of the changes in this PR. Specifically initNum, initSeq := fc.accountNumber, fc.sequence
- if initNum == 0 || initSeq == 0 {
+ if initNum == 0 && initSeq == 0 {
num, seq, err := fc.accountRetriever.GetAccountNumberSequence(clientCtx, from)
if err != nil {
return fc, err
}
- if initNum == 0 {
- fc = fc.WithAccountNumber(num)
- }
+ fc = fc.WithAccountNumber(num)
+ fc = fc.WithSequence(seq)
-
- if initSeq == 0 {
- fc = fc.WithSequence(seq)
- }
}In interchain test, we were accidentally not explicitly passing the account sequence when creating the tx Factory. As a result, the I will create a follow up PR in interchain test to fix this issue there. I believe there should be a check in the sdk that ensures if exactly one of the account / account sequence is zero, it should error out. |
| cdc := Codec() | ||
| return clientContext.WithCodec(cdc).WithTxConfig(authtx.NewTxConfig(cdc, []signingtypes.SignMode{signingtypes.SignMode_SIGN_MODE_DIRECT})) | ||
| }) | ||
|
|
||
| broadcaster.ConfigureFactoryOptions(func(factory tx.Factory) tx.Factory { | ||
| return factory.WithGas(DefaultGasValue) | ||
| return factory.WithGas(DefaultGasValue).WithSequence(seq) |
| // this is a temporary work around to pass the acc sequence correctly. | ||
| // TODO: create PR against interchain test to make this the default behaviour. | ||
| sdkAdd, err := sdk.AccAddressFromBech32(user.FormattedAddress()) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
||
| _, seq, err = clientContext.AccountRetriever.GetAccountNumberSequence(clientContext, sdkAdd) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
|
|
There was a problem hiding this comment.
this can be removed when this issue is resolved.
There was a problem hiding this comment.
dope working on debugging the issue @chatton & @charleenfei
|
Fantastic work y'all! |
(cherry picked from commit 87ac1a9) # Conflicts: # e2e/go.mod # e2e/go.sum # e2e/testsuite/tx.go # go.mod # go.sum # modules/capability/go.mod # modules/capability/go.sum
Description
Waiting on this PR in interchaintest to be merged.
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.