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: reverse search links #793
feat: reverse search links #793
Changes from 21 commits
c2d9106
45ed167
9cdd3b0
ff758d7
f65bcfa
9afd731
982826c
a3a26f6
90549f7
a1f6d38
76e26d8
2a362e4
ab71a63
c643093
4bc9c28
996e2f8
6ea38bc
6fea2ff
a55649a
63fedef
9c48b8d
a51b8b9
ce789c3
3ee9064
6fd74df
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
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 it can be all implemented by
bytes.Split
instead ofbytes.TrimSuffix
, say:Then, the value of the key can be simply
0x01
in order to reduce the storage on chain.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.
Right now, links owners are stored using the following keys:
So, if we use the
bytes.Split
it would return the following values:We would have to remove the
User address
suffix anyway to get the external address.Also, the key for chain link owners right now are:
Which already are 1 byte each. We already use
0x01
for theIBCPortKey
key. We can't re-use that. Also,0x01
and0x16
are both 1 byte each so that wouldn't impact the chain storage spaceThere 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.
@RiccardoM Sorry for that the explanation was not clear, I mean that we can store the key like:
Then the
bytes.Split
would return the values:As a result, the storage doesn't need to store user address as value again.
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.
@dadamu You are completely right! Thanks for pointing that out, this way we can save up a lot of disk space. I've just implemented it this way (and also added some missing tests)
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.
Considering this answer, should we look after other places where we use
bytes.TrimSuffix
andbytes.TrimPrefix
and see if we can improve them as well?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.
@bragaz All other keys instances do not trim prefixes unless necessary (eg. inside user group members). All the keys have already been reviewed inside their own PRs before being merged, and I don't think it would be appropriate to do it here anyway. I've checked though and didn't find any use of
bytes.TrimPrefix
aside from this case and user groups members anyway