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

ICS33: Multi-hop Channel Spec #882

Merged
merged 52 commits into from
Sep 14, 2023
Merged
Changes from 1 commit
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
fb716fc
feat: add spec for multi-hop channels
notbdu Nov 11, 2022
3bc1800
chore: naming
notbdu Nov 11, 2022
f7f1a6c
chore: add notes on chain registry
notbdu Nov 11, 2022
1421d33
Polymer/multihop (#1)
dshiell Dec 16, 2022
2be7894
update to ics-033
dshiell Dec 16, 2022
eb0193c
start updating channel spec for multihop; add diagram for relayer pro…
dshiell Dec 17, 2022
3e167eb
chore: update spec history
notbdu Dec 17, 2022
f773339
chore: spacing
notbdu Dec 17, 2022
437c6f9
remove TODO comment; per PR feedback, connection proof not require fo…
dshiell Dec 20, 2022
337d393
add comment to MultihopProof struct making the proof ordering more cl…
dshiell Dec 20, 2022
4d62c51
add verification proof pseudo code
dshiell Dec 20, 2022
280fc28
add/update multihop proof verification logic
dshiell Dec 20, 2022
c1f8bd0
Update spec/core/ics-004-channel-and-packet-semantics/README.md
dshiell Feb 13, 2023
6dcb58a
Update spec/core/ics-033-multi-hop/README.md
dshiell Feb 13, 2023
bb4f37e
remove consState var to simplify consensusState update assignment
dshiell Feb 13, 2023
1af38b5
rename VerifyMultihopConsensusStateProof --> VerifyMultihopConsensusA…
dshiell Feb 14, 2023
fc34fdf
simplify multihop proof logic steps further
dshiell Feb 14, 2023
ebc08ee
Ds/multihop (#4)
dshiell Feb 15, 2023
9ee66b2
start updating multihop spec to include client state information in t…
dshiell Feb 17, 2023
a7ecb58
check consensus/connection proof lengths immediately
dshiell Feb 24, 2023
87795f8
update spec with latest multihop solution
dshiell Mar 1, 2023
eca9ded
remove client state from multihop proofs
dshiell Mar 8, 2023
44e4ac1
update spec with chanCloseFrozen; fix/update other parts of multihop …
dshiell Mar 8, 2023
c85c8aa
revise and update multihop spec details
dshiell Mar 8, 2023
7f133d7
add check to match client id in ConnectionEnd with client id in subse…
dshiell Mar 11, 2023
1213e61
fix indentation
dshiell Mar 11, 2023
3784cef
Update spec/core/ics-033-multi-hop/README.md
dshiell Mar 14, 2023
bd6c39e
Update spec/core/ics-033-multi-hop/README.md
dshiell Mar 14, 2023
f4baac5
Update spec/core/ics-033-multi-hop/README.md
dshiell Mar 14, 2023
c04a201
Update spec/core/ics-004-channel-and-packet-semantics/README.md
dshiell Mar 14, 2023
5e0d503
Update spec/core/ics-004-channel-and-packet-semantics/README.md
dshiell Mar 14, 2023
4f91306
Update spec/core/ics-004-channel-and-packet-semantics/README.md
dshiell Mar 14, 2023
17da31d
fix typo prevChain --> previousChain
dshiell Mar 14, 2023
594a5bb
add comments for connection and multihopConnectionEnd
dshiell Mar 15, 2023
12e7ad5
Update spec/core/ics-003-connection-semantics/README.md
dshiell Mar 16, 2023
9265373
add frozen channel proof generation logic
dshiell Mar 15, 2023
dd84773
refactor getMaxDelayPeriod to return the time and block delay
dshiell Mar 16, 2023
4dd2204
use Identifier type instead of string for connectionHops array
dshiell Mar 16, 2023
243bd2e
cleanup proof gen pseudo code
dshiell Mar 16, 2023
804a828
check proof heights for consensus and connection states during multih…
dshiell Mar 17, 2023
b9ca4ca
Update spec/core/ics-033-multi-hop/README.md
dshiell Mar 17, 2023
9f13123
cleanup multihop proof gen pseudo code
dshiell Mar 17, 2023
7bebf3b
remove trailing whitespace; refactor multihop proof generation and ad…
dshiell Mar 21, 2023
f69e191
add extra info about connectionHops ordering; update proof ordering t…
dshiell Mar 22, 2023
56e0f0a
clarify multi-hop proof verification now that proof ordering matches …
dshiell Mar 24, 2023
73ef434
address latest PR feedback
dshiell Apr 11, 2023
4262a63
more pr feedback
dshiell Apr 11, 2023
6f52039
address more multihop PR feedback
dshiell Apr 19, 2023
c76b25a
update multi-hop proof query/verification specs and add frozen channe…
dshiell Jun 28, 2023
d0024c6
update multi-hop proof diagrams
dshiell Jun 29, 2023
e385e8d
wip address latest PR feedback
dshiell Jul 28, 2023
071b4ed
add more clarifying comments/explanations and incorporate more PR fee…
dshiell Aug 1, 2023
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
Prev Previous commit
Next Next commit
update spec with latest multihop solution
dshiell committed Aug 2, 2023
commit 87795f81ceafb774a621adc2e81d79bd7d148ee6
Binary file added spec/core/.DS_Store
Binary file not shown.
64 changes: 46 additions & 18 deletions spec/core/ics-033-multi-hop/README.md
Original file line number Diff line number Diff line change
@@ -262,18 +262,20 @@ func GenerateClientStateProofs(chains []*Chain) []*ProofData {

The following outlines the general proof verification steps specific to a multi-hop IBC message.

1. Unpack the multihop proof bytes into consensus, connection, and channel/commitment proof data.
2. Iterate through connection proof data and verify each connectionEnd is in the OPEN state. Check connectionHops during channel handshake.
3. Iterate through each clientState and verify the status is active and the clientID matches the clientID for the corresponding consensusState.
4. Starting with known `ConsensusState[N-1]` at the given `proofHeight` on `ChainN` prove the prior chain's consensus, clientState, and connection state.
5. Repeat step 3, proving `ConsensusState[i-1]`, `ClientState[i-1]`, and `Conn[i-1,i]` until `ConsensusState[0]` on the source chain is proven.
1. Unpack the multihop proof bytes into consensus states, connection states, client states and channel/commitment proof data.
2. Check the counterparty client on the receiving end is active and the client height is greater than or equal to the proof height.
3. Iterate through the connections states to determine the maximum delayPeriod for the channel path and verify that the counterparty consensus state on the receiving chain satisfies the delay requirement.
4. Iterate through connection state proofs and verify each connectionEnd is in the OPEN state and check that the connection ids match the channel connectionHops.
5. Iterate through each client state and verify that no clients are frozen and the clientID matches the clientID for the corresponding consensus state.
6. Verify the intermediate state proofs. Starting with known `ConsensusState[N-1]` at the given `proofHeight` on `ChainN` prove the prior chain's consensus, clientState, and connection state.
7. Repeat step 3, proving `ConsensusState[i-1]`, `ClientState[i-1]`, and `Conn[i-1,i]` until `ConsensusState[0]` on the source chain is proven.
- Start with i = N-1
- ConsensusState <= ConsensusState[i-1] on Chain[i]
Copy link
Member

Choose a reason for hiding this comment

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

Since we have access to all the intermediate consensus states, we should definitely check that the timestamp in the consensus state is still within the trusting period of the final client. This way we can ensure we are using fresh state

- ConsensusProofs[i].Proof.VerifyMembership(ConsensusState.GetRoot(), ConsensusProofs[i].Key, ConsensusProofs[i].Value)
- ConnectionProofs[i].Proof.VerifyMembership(ConsensusState.GetRoot(), ConnectionProofs[i].Key, ConnectionProofs[i].Value)
- ClientStateProofs[i].Proof.VerifyMembership(ConsensusState.GetRoot(), ClientStateProofs[i].Key, ClientStateProofs[i].Value)
- Set ConsensusState from unmarshalled ConsensusProofs[i].Value
6. Finally, prove the expected channel or packet commitment in `ConsensusState[0]`
8. Finally, prove the expected channel or packet commitment in `ConsensusState[0]`

For more details see [ICS4](https://github.com/cosmos/ibc/tree/main/spec/core/ics-004-channel-and-packet-semantics).

@@ -300,16 +302,21 @@ func VerifyMultihopProof(
// deserialize proof bytes into multihop proofs
proofs := abortTransactionUnless(Unmarshal(proof))
abortTransactionUnless(len(proofs.ConsensusProofs) >= 1)
abortTransactionUnless(len(proofs.ConnectionProofs) >= 1)
abortTransactionUnless(len(proofs.ClientProofs) >= 1)
abortTransactionUnless(len(proofs.ConnectionProofs) == len(proofs.ConsensusProofs))

// verify connection states and ordering
abortTransactionUnless(VerifyConnectionStates(proofs.ConnectionProofs, connectionHops))

// verify intermediate consensus and connection states from destination --> source
abortTransactionUnless(VerifyMultiHopConsensusAndConnectionStateProofs(consensusState, proofs.ConsensusProofs, proofs.ConnectionProofs))
// verify client states are not frozen
abortTransactionUnless(VerifyClientStates(proofs.ClientStates))

// verify the keyproof on source chain's consensus state.
abortTransactionUnless(VerifyMultiHopKeyProof(proofs, prefix, key, value))
// verify intermediate consensus, connection, and client states from destination --> source
abortTransactionUnless(VerifyIntermediateStateProofs(consensusState, proofs.ConsensusProofs, proofs.ConnectionProofs, proofs.ClientProofs))

// verify a key/value proof on source chain's consensus state.
abortTransactionUnless(VerifyKeyValueProof(proofs, prefix, key, value))
}

// VerifyConnectionStates checks that each connection in the multihop proof is OPEN and matches the connections in connectionHops.
@@ -331,18 +338,29 @@ func VerifyConnectionStates(
}
}

// VerifyMultiHopConsensusAndConnectionStateProofs verifies the state of each intermediate consensus and
// connection state starting from chain[N-1] on the destination (chain[N]) and finally proving the source
// chain consensus and connection state.
func VerifyMultiHopConsensusAndConnectionStateProofs(
// Verify ClientStates checks that each client in the proof chain is not frozen.
func VerifyClientStates(
clientStateProofs []*MultihopProof,
) {
for i, data := range clientStateProofs {
clientState := abortTransactionUnless(Unmarshal(data.Value))
abortTransactionUnless(clientState.CheckFrozen() == false)
}
}

// VerifyIntermediateStateProofs verifies the state of each intermediate consensus, connection, and
// client state starting from chain[N-1] on the destination (chain[N]) and finally proving the source
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// client state starting from chain[N-1] on the destination (chain[N]) and finally proving the source
// client state starting from receiving (chain[0]) and finally proving the source

// chain consensus, connection, and client state.
func VerifyIntermediateStateProofs(
consensusState exported.ConsensusState,
consensusProofs []*MultihopProof,
connectionProofs []*MultihopProof,
clientProofs []*MultihopProof,
) {
// reverse iterate through proofs to prove from destination to source
for i := len(consensusProofs) - 1; i >= 0; i-- {

// prove the consensus state of chain[i] in chain[i+1]
// prove the consensus state of chain[i] on chain[i+1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// prove the consensus state of chain[i] on chain[i+1]
// prove the consensus state of chain[i+1] on chain[i]

Copy link
Contributor

Choose a reason for hiding this comment

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

made this chain[i] on chain[i-1] to match surrounding proof indexing

consensusProof := abortTransactionUnless(Unmarshal(consensusProofs[i].Proof))
abortTransactionUnless(consensusProof.VerifyMembership(
commitmenttypes.GetSDKSpecs(),
Copy link
Contributor

Choose a reason for hiding this comment

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

One question:
Does this assume that all proof specs for membership verification for each chain are equal? If not, then I think we need to refer to the client state as well as the consensus state for each chain.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah currently it is assuming proof specs are equal.

Copy link
Contributor

Choose a reason for hiding this comment

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

imo this is a very important point and we should make it very clear in the spec.

@@ -351,7 +369,7 @@ func VerifyMultiHopConsensusAndConnectionStateProofs(
consensusProof.Value,
))

// prove the connection state of chain[i] in chain[i+1]
// prove the connection state of chain[i] on chain[i+1]
connectionProof := abortTransactionUnless(Unmarshal(connectionProofs[i].Proof))
abortTransactionUnless(connectionProof.VerifyMembership(
commitmenttypes.GetSDKSpecs(),
@@ -360,13 +378,23 @@ func VerifyMultiHopConsensusAndConnectionStateProofs(
connectionProof.Value,
))

// prove the client state of chain[i] on chain[i+1]
clientProof := abortTransactionUnless(Unmarshal(clientProofs[i].Proof))
abortTransactionUnless(clientProof.VerifyMembership(
commitmenttypes.GetSDKSpecs(),
consensusState.GetRoot(),
*clientProof.PrefixedKey,
clientProof.Value,
))


// update the consensusState to chain[i] to prove the next consensus/connection states
consensusState = abortTransactionUnless(UnmarshalInterface(consensusProof.Value))
}
}

// VerifyMultiHopKeyProof verifies a key in the source chain consensus state.
func VerifyMultiHopKeyProof(
// VerifyKeyValueProof verifies a key in the source chain consensus state.
func VerifyKeyValueProof(
proofs *MsgMultihopProof,
prefix exported.Prefix,
key string,