-
Notifications
You must be signed in to change notification settings - Fork 121
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
chore: update namada to 0.24.0 #416
Conversation
6d2c8f5
to
6e9b5ff
Compare
827999a
to
b1eba77
Compare
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!
@mateuszjasiuk should we wait to merge this until those changes are in the next release? If it is blocking anything else, I think we could merge it sooner if needed.
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.
Looks good to me and the transaction code is much cleaner now 👍
I think the CI is failing due to some naming error, and one minor comment, but everything looks good!
@@ -288,34 +287,17 @@ impl Sdk { | |||
tx_msg: &[u8], | |||
password: Option<String>, | |||
xsk: Option<String>, | |||
gas_payer: Option<String> | |||
_gas_payer: Option<String>, |
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.
I guess these gas_payer
parameters can be completely removed now.
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.
I think we can do it later, as there is a lot to do anyways :)
7de692d
to
34bfb7d
Compare
34bfb7d
to
fce6778
Compare
NOTE:
For some reason fetched wasm checksums are not correct :/
If you want to test this using e2e tests:
e2e/.namada
e2e/.namada/wasm