Skip to content

Flexible identifier selection #482

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
Oct 2, 2020
Merged

Flexible identifier selection #482

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
9ef11e5
Modify ICS 4 (channel handshake) for flexible identifier
cwgoes Sep 15, 2020
2e0c350
Modify ICS 3 (connection handshake) for flexible identifier
cwgoes Sep 15, 2020
262a7de
Merge branch 'master' into cwgoes/fixed-variables-fields
cwgoes Sep 28, 2020
5d845a0
Add comments
cwgoes Sep 28, 2020
67d9088
Update; rebuild
cwgoes Sep 28, 2020
b805be3
Check that we can support version
cwgoes Oct 1, 2020
4630d60
Add comment
cwgoes Oct 2, 2020
ff93ac3
Apply suggestions from code review
cwgoes Oct 2, 2020
3354f39
Add desiredIdentifier function
cwgoes Oct 2, 2020
dad036a
Rebuild spec
cwgoes Oct 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified spec.pdf
View file
Open in desktop
Binary file not shown.
51 changes: 38 additions & 13 deletions spec/ics-003-connection-semantics/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,13 @@ At the end of an opening handshake between two chains implementing the sub-proto

This sub-protocol need not be permissioned, modulo anti-spam measures.

In `connOpenInit`, a sentinel empty-string identifier can be used to allow the recipient chain to choose its own connection identifier. Chains may implement a function `desiredIdentifier` which chooses an identifier, e.g. by incrementing a counter:

```typescript
type desiredIdentifier = (provedIdentifier: Identifier) -> Identifier
```

A specific version can optionally be passed as `version` to ensure that the handshake will either complete with that version or fail.

*ConnOpenInit* initialises a connection attempt on chain A.

Expand All @@ -290,12 +297,20 @@ function connOpenInit(
desiredCounterpartyConnectionIdentifier: Identifier,
counterpartyPrefix: CommitmentPrefix,
clientIdentifier: Identifier,
counterpartyClientIdentifier: Identifier) {
counterpartyClientIdentifier: Identifier,
version: string) {
abortTransactionUnless(validateConnectionIdentifier(identifier))
abortTransactionUnless(provableStore.get(connectionPath(identifier)) == null)
state = INIT
if version != "" {
// manually selected version must be one we can support
abortTransactionUnless(getCompatibleVersions().indexOf(version) > -1)
versions = [version]
} else {
versions = getCompatibleVersions()
}
connection = ConnectionEnd{state, desiredCounterpartyConnectionIdentifier, counterpartyPrefix,
clientIdentifier, counterpartyClientIdentifier, getCompatibleVersions()}
clientIdentifier, counterpartyClientIdentifier, versions}
provableStore.set(connectionPath(identifier), connection)
addConnectionToClient(clientIdentifier, identifier)
}
Expand All @@ -306,6 +321,7 @@ function connOpenInit(
```typescript
function connOpenTry(
desiredIdentifier: Identifier,
provedIdentifier: Identifier,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couldn't this just be a boolean indicating if the sending chain decided to choose the identifier or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, though we'll still construct a "provedIdentifier" since we need to check it

counterpartyConnectionIdentifier: Identifier,
counterpartyPrefix: CommitmentPrefix,
counterpartyClientIdentifier: Identifier,
Expand All @@ -318,24 +334,27 @@ function connOpenTry(
abortTransactionUnless(validateConnectionIdentifier(desiredIdentifier))
abortTransactionUnless(consensusHeight < getCurrentHeight())
expectedConsensusState = getConsensusState(consensusHeight)
expected = ConnectionEnd{INIT, desiredIdentifier, getCommitmentPrefix(), counterpartyClientIdentifier,
abortTransationUnless(
provedIdentifier === desiredIdentifier ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit confusing to me.

proved identifier is empty or comes from the init chain.

Desired Identifier comes from the receiving chain. But where? There doesn't seem to be a way to say "use whatever name they suggest", or "use whatever name they suggest if it starts with 'foo'" for example.

Rather than comparing here, it would make more sense to have

function desiredIdentifier(provedIdentifier) -> Identifier {}

So the receiving module can make this decision and about if desired

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that's the intended usage, it just doesn't need to happen "within" the IBC module, per se. I can add a note about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment with your example.

provedIdentifier === ""
)
expected = ConnectionEnd{INIT, provedIdentifier, getCommitmentPrefix(), counterpartyClientIdentifier,
clientIdentifier, counterpartyVersions}
versionsIntersection = intersection(counterpartyVersions, getCompatibleVersions())
version = pickVersion(versionsIntersection)
connection = ConnectionEnd{TRYOPEN, counterpartyConnectionIdentifier, counterpartyPrefix,
clientIdentifier, counterpartyClientIdentifier, version}
abortTransactionUnless(connection.verifyConnectionState(proofHeight, proofInit, counterpartyConnectionIdentifier, expected))
abortTransactionUnless(connection.verifyClientConsensusState(
proofHeight, proofConsensus, counterpartyClientIdentifier, consensusHeight, expectedConsensusState))
previous = provableStore.get(connectionPath(desiredIdentifier))
abortTransactionUnless(
(previous === null) ||
(previous.state === INIT &&
previous.counterpartyConnectionIdentifier === counterpartyConnectionIdentifier &&
previous.counterpartyPrefix === counterpartyPrefix &&
previous.clientIdentifier === clientIdentifier &&
previous.counterpartyClientIdentifier === counterpartyClientIdentifier &&
previous.version === getCompatibleVersions()))
previous.counterpartyClientIdentifier === counterpartyClientIdentifier))
versionsIntersection = intersection(counterpartyVersions, previous !== null ? previous.version : getCompatibleVersions())
version = pickVersion(versionsIntersection) // throws if there is no intersection
connection = ConnectionEnd{TRYOPEN, counterpartyConnectionIdentifier, counterpartyPrefix,
clientIdentifier, counterpartyClientIdentifier, version}
abortTransactionUnless(connection.verifyConnectionState(proofHeight, proofInit, counterpartyConnectionIdentifier, expected))
abortTransactionUnless(connection.verifyClientConsensusState(
proofHeight, proofConsensus, counterpartyClientIdentifier, consensusHeight, expectedConsensusState))
identifier = desiredIdentifier
provableStore.set(connectionPath(identifier), connection)
addConnectionToClient(clientIdentifier, identifier)
Expand All @@ -348,24 +367,30 @@ function connOpenTry(
function connOpenAck(
identifier: Identifier,
version: string,
counterpartyIdentifier: Identifier,
proofTry: CommitmentProof,
proofConsensus: CommitmentProof,
proofHeight: Height,
consensusHeight: Height) {
abortTransactionUnless(consensusHeight < getCurrentHeight())
connection = provableStore.get(connectionPath(identifier))
abortTransactionUnless(
counterpartyIdentifier === connection.counterpartyConnectionIdentifier ||
connection.counterpartyConnectionIdentifier === ""
)
abortTransactionUnless(
(connection.state === INIT && connection.version.indexOf(version) !== -1)
|| (connection.state === TRYOPEN && connection.version === version))
expectedConsensusState = getConsensusState(consensusHeight)
expected = ConnectionEnd{TRYOPEN, identifier, getCommitmentPrefix(),
connection.counterpartyClientIdentifier, connection.clientIdentifier,
version}
abortTransactionUnless(connection.verifyConnectionState(proofHeight, proofTry, connection.counterpartyConnectionIdentifier, expected))
abortTransactionUnless(connection.verifyConnectionState(proofHeight, proofTry, counterpartyIdentifier, expected))
abortTransactionUnless(connection.verifyClientConsensusState(
proofHeight, proofConsensus, connection.counterpartyClientIdentifier, consensusHeight, expectedConsensusState))
connection.state = OPEN
connection.version = version
connection.counterpartyConnectionIdentifier = counterpartyIdentifier
provableStore.set(connectionPath(identifier), connection)
}
```
Expand Down
19 changes: 17 additions & 2 deletions spec/ics-004-channel-and-packet-semantics/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,8 @@ When the opening handshake is complete, the module which initiates the handshake
it specifies will own the other end of the created channel on the counterparty chain. Once a channel is created, ownership cannot be changed (although higher-level abstractions
could be implemented to provide this).

A sentinel empty-string identifier can be used to allow the recipient chain to choose its own channel identifier.

```typescript
function chanOpenInit(
order: ChannelOrder,
Expand Down Expand Up @@ -311,6 +313,7 @@ function chanOpenTry(
connectionHops: [Identifier],
portIdentifier: Identifier,
channelIdentifier: Identifier,
provedIdentifier: Identifier,
counterpartyPortIdentifier: Identifier,
counterpartyChannelIdentifier: Identifier,
version: string,
Expand All @@ -319,6 +322,11 @@ function chanOpenTry(
proofHeight: Height): CapabilityKey {
abortTransactionUnless(validateChannelIdentifier(portIdentifier, channelIdentifier))
abortTransactionUnless(connectionHops.length === 1) // for v1 of the IBC protocol
// empty-string is a sentinel value for "allow any identifier"
abortTransationUnless(
provedIdentifier === channelIdentifier ||
provedIdentifier === ""
)
previous = provableStore.get(channelPath(portIdentifier, channelIdentifier))
abortTransactionUnless(
(previous === null) ||
Expand All @@ -334,7 +342,7 @@ function chanOpenTry(
abortTransactionUnless(connection !== null)
abortTransactionUnless(connection.state === OPEN)
expected = ChannelEnd{INIT, order, portIdentifier,
channelIdentifier, [connection.counterpartyConnectionIdentifier], counterpartyVersion}
provedIdentifier, [connection.counterpartyConnectionIdentifier], counterpartyVersion}
abortTransactionUnless(connection.verifyChannelState(
proofHeight,
proofInit,
Expand All @@ -361,11 +369,17 @@ function chanOpenAck(
portIdentifier: Identifier,
channelIdentifier: Identifier,
counterpartyVersion: string,
counterpartyChannelIdentifier: string,
proofTry: CommitmentProof,
proofHeight: Height) {
channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
abortTransactionUnless(channel.state === INIT || channel.state === TRYOPEN)
abortTransactionUnless(authenticateCapability(channelCapabilityPath(portIdentifier, channelIdentifier), capability))
// empty-string is a sentinel value for "allow any identifier"
abortTransactionUnless(
counterpartyChannelIdentifier === channel.counterpartyChannelIdentifier ||
channel.counterpartyChannelIdentifier === ""
)
connection = provableStore.get(connectionPath(channel.connectionHops[0]))
abortTransactionUnless(connection !== null)
abortTransactionUnless(connection.state === OPEN)
Expand All @@ -375,11 +389,12 @@ function chanOpenAck(
proofHeight,
proofTry,
channel.counterpartyPortIdentifier,
channel.counterpartyChannelIdentifier,
counterpartyChannelIdentifier,
expected
))
channel.state = OPEN
channel.version = counterpartyVersion
channel.counterpartyChannelIdentifier = counterpartyChannelIdentifier
provableStore.set(channelPath(portIdentifier, channelIdentifier), channel)
}
```
Expand Down