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
feat: Add queries for PSS and consumer commission rate #1733
feat: Add queries for PSS and consumer commission rate #1733
Changes from 27 commits
b0d4cf9
0162242
7ad2993
5cd5ba5
5393f19
03af237
07429fa
0ab0b32
1a0fa9e
56ca38d
c614fd2
6503f9b
330c085
629cfd1
afd4652
2e95f89
a9db491
1e3a8e7
149ba7a
24be8b2
b311f35
22ca561
5509108
ad91b0e
559b28f
00e43a9
4f9084f
d4fc4cb
938d930
9739ddd
c01ab08
4661a99
219d310
10dbb53
d642e8a
69d0b28
f9e63f0
3ae3031
ed402f8
c75594f
899f3ea
b1c1beb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Why do we need this? (it's more clear for users is a perfectly fine reason if you think it applies)
Can't we keep the TopN == 0 means it's an opt-in chain, and just have the TopN field?
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.
We don't need it in the logic. The intention was to make it clearer for the user. But after some thought, it can actually be removed.
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.
Could we make this a bit more explicit?
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 this should be:
GetTopN
would return afound
on whetherTopN
is set, butTopN
could be set to0
.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.
Similarly, below in
keeper_test.go
you probably won't need to checkif topN > 0 {
inThere 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.
Make sense, thanks. Updated.