Skip to content
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

storage(substore): add delimiter handling #3441

Merged
merged 3 commits into from
Nov 28, 2023
Merged

Conversation

erwanor
Copy link
Member

@erwanor erwanor commented Nov 28, 2023

According to the ICS24 spec, substore prefixes are identifiers. This means that they "MUST" follow precise requirements:

An Identifier is a bytestring used as a key for an object stored in state, such as a connection, channel, or light client.

Identifiers MUST be non-empty (of positive integer length).

Identifiers MUST consist of characters in one of the following categories only:

Alphanumeric
., _, +, -, #
[, ], <, >

This means that our decision to include a trailing delimiter for our IBC prefix is technically incompatible with the spec. This is unfortunate, because it greatly simplifies the routing logic. However, in practice this could become a problem if a counterparty IBC implementation decides to enforce this requirement.

This PR adds support for removing delimiters from our defined prefixes. Instead of defining an ibc/ prefix, the user defines a prefix identifier "ibc". The routing logic handles adding/stripping a delimiter "/". For example, a key path: ibc/path/to/client/state will result in a match to the ibc substore at key path/to/client/state.

However, quietly managing delimiters adds some complexity. Naively, we could find ourselves in a situation where we collisions occur e.g. "ibcpath/to/client/state" and ibc/path/to/client/state. Similarly, we now have extra edge cases to enforce that we never write an empty key to a substore: ibc, ibc/ would resolve to the same key.

To avoid this, we only route a key to a substore if it contains a prefix, a delimiter, and a non-empty substore path e.g. "ibc/path/to" and "ibc/key" routes to the "ibc" substore, but "ibc/" and "ibc" don't.

@erwanor
Copy link
Member Author

erwanor commented Nov 28, 2023

I'll change our StateWrite trait to forbid inserting empty keys in a future PR.

@avahowell
Copy link
Contributor

LGTM

@erwanor erwanor merged commit 79c30a4 into main Nov 28, 2023
6 checks passed
@erwanor erwanor deleted the erwan/ics24_delimiters branch November 28, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants