Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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(types)!: removed global config for verifier #18607
refactor(types)!: removed global config for verifier #18607
Changes from all commits
5445533
bbb175c
42213bc
99deec0
27e62ed
f190645
272083d
a3070eb
016259c
28344a3
ae0dbc8
2cdd14c
e3bd0d2
ce6e6e8
999633c
04b2a7a
545c053
d906067
e5d6193
246aedb
21d6c7d
e21d6c0
13d1ce9
b5138da
de01090
88ef673
0f210e3
4f78965
1d30858
a4f22ac
65bfcc3
a4ea39c
a3b5ba1
33a826d
0b45fd6
84e8efb
47b0bc2
e6c304c
c59f4b1
17b77b9
1094070
6f6c668
56beacc
1a0272e
11adeae
26f6ad2
333a0d1
696d40f
37b83f6
bffaa0e
71f9d5a
5c6e148
f7f2691
1db671b
ec7daa8
9d3c7e9
d15a92e
ae6fc3b
d9437c5
83dc83b
4d8bf97
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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 need to keep this function and the verification in other address functions as not everyone will migrate right away. We would remove functionality to set custom verifiers but verify the basic we do 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.
People can use an address codec to do so tbh. It is being populated everywhere in v0.51 (clientCtx, sign mode context, keepers).
Keeping it around could lead to a divergence of the address codec verification and the one in the global config.
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 do see a reason to have a verification function as @tac0turtle says, but I also think @julienrbrt is right I think it is redundant that users could send their own verification function. Maybe adding the verification function to the
Address
interface would solve the issueThere 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.
The refactoring to use
addresscodec.NewBech32Codec
for address decoding introduces a significant change in how addresses are handled within the SDK. It's important to ensure that documentation, examples, and migration guides are updated to reflect these changes. This will help developers understand how to use the new address codecs and adjust their code accordingly.Would you like assistance in updating the documentation or creating a migration guide for these changes?