-
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
Query issue when upgrading from v0.37.3 to v0.37.8 #5964
Comments
I just made an attempt with the newest version of Gaia, and it seems the problem is also present there. To reproduce:
The issue can now be demonstrated with the following queries:
Provides expected output with tx details
Crashes with a stacktrace that is very similar to the one in the issue. |
@jgimeno do you think you have the bandwidth to tackle this? |
Gonna check! |
@alexanderbez the problem is that 0.37.x does not include this bugfix. An option can be to upgrade to 0.38 if possible. Another one is to release a new 0.37 with this included. |
I don't think that is a true fix to the issue, but we should certainly cherry-pick that into the next 0.37.x release. This was working before -- which leads me to believe this has something to do with Viper. |
More information, from Gaia v2.0.4 stopped working. It included these changes: (sdk) Bump SDK version to v0.37.5. That includes internally too a change from viper 1.4.0 to 1.6.1 |
Ok I know where is the issue, viper 1.6.0 has this feature
cosmos-sdk/client/context/context.go Line 108 in 6d5ca0b
|
Gosh, this breaks quite few things. @jgimeno and I are on the case. |
We'll have to change the pruning flags, their parsing heavily relies on |
Would it be an option to downgrade the Viper dependency? Is there something essential in the 1.6 release? |
In line of principle, I'm not against downgrading to viper 1.4 - my immediate, natural reaction was to do so. Afterwards, we decided to invest some time in trying to fix the use of |
Yeah, the problem is that it means that cobra will need to be downgraded too. |
Why? Can you not just simply use the replace directive? |
Yes, sure we could. We are trying to fix the problem first though. |
Experiencing this too when running gaiacli queries on Gaia v2.0.8.
|
@franono this should solve it b754065 EDIT: actually I can't reproduce that @franono:
|
Nevermind @franono. If I remove the |
github.com/spf13/viper's recent releases introduced a semantic change in some public API such as viper.IsSet(), which have broken some of our flags checks. Instead of checking whether users have changed a flag's default value we should rely on such defaults and adjust runtime behaviour accordingly. In order to do so, it's important that we pick sane defaults for all our flags. The --pruning flag and configuration option now allow for a fake custom strategy. When users elect custom, then the pruning-{keep,snapshot}-every options are interpreted and parsed; else they're ignored. Zero is pruning-{keep,snapshot}-every default value. When users choose to set a custom pruning strategy they are signalling that they want more fine-grainted control, therefore it's legitimate to expect them to know what they are doing and enter valid values for both options. Ref #5964
This is not solved in the v0.37.x series of cosmos-sdk. Hence reopening. |
Fixed in cosmos-sdk v0.37.10. Hence closing. |
We need to release this too #6021. Hence reopening. |
This is now fixed in cosmos-sdk v0.37.11. Hence closing. |
Summary of Bug
After upgrading, I can't query by tx hash unless I specify the
--trust-node
flag.Not specifying the flag results in the following stack trace:
Version
Cosmos-SDK v0.37.8
Possible fix
Adding the line
viper.SetDefault(flags.FlagTrustNode, "false")
in the
main
function of the cli seems to fix the issue. Could it be an issue with the updated Viper library?Steps to Reproduce
Checkout our the following em-ledger commit: e-money/em-ledger@d2db681
make build
./build/emcli q tx 2B08FC02FA89169C28056867FEB58423F0EA2DB9AC4F43F5BC5098C271E9BF69 --node tcp://emoney.validator.network:80
For Admin Use
The text was updated successfully, but these errors were encountered: