From 6281f83be144fd3cc6dace99d9928f6fc35008cd Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 15 Aug 2022 17:49:34 +0200 Subject: [PATCH 01/11] update solomachine for 02-client-refactor --- .../ics-006-solo-machine-client/README.md | 255 +++++++----------- 1 file changed, 103 insertions(+), 152 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 00932917b..ae6f3c2e2 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -78,6 +78,20 @@ interface Header { } ``` +### Signature Verification + +The solomachine public key must sign over the following struct: + +```typescript +interface SignBytes { + sequence: uint64 + timestamp: uint64 + diversifier: string + path: []byte + data: []byte +} +``` + ### Misbehaviour `Misbehaviour` for solo machines consists of a sequence and two signatures over different messages at that sequence. @@ -85,7 +99,9 @@ interface Header { ```typescript interface SignatureAndData { sig: Signature + path: Path data: []byte + timestamp: Timestamop } interface Misbehaviour { @@ -129,201 +145,135 @@ function latestClientHeight(clientState: ClientState): uint64 { ### Validity predicate -The solo machine client `checkValidityAndUpdateState` function checks that the currently registered public key has signed over the new public key with the correct sequence. +The solo machine client `verifyClientMessage` function checks that the currently registered public key and diversifier signed over the client message at the expected sequence. If the client message is an update, then it must be the current sequence. If the client message is misbehaviour then it must be the sequence of the misbehaviour. ```typescript -function checkValidityAndUpdateState( +function verifyClientMessage( clientState: ClientState, - header: Header) { - assert(header.sequence === clientState.consensusState.sequence) - assert(header.timestamp >= clientstate.consensusState.timestamp) - assert(checkSignature(header.newPublicKey, header.sequence, header.diversifier, header.signature)) - clientState.consensusState.publicKey = header.newPublicKey - clientState.consensusState.diversifier = header.newDiversifier - clientState.consensusState.timestamp = header.timestamp - clientState.consensusState.sequence++ + clientMsg: ClientMessage) { + switch typeof(ClientMessage) { + case Header: + assert(header.sequence === clientState.consensusState.sequence) + assert(header.timestamp >= clientstate.consensusState.timestamp) + assert(checkSignature(header.newPublicKey, header.sequence, header.diversifier, header.signature)) + // misbehaviour only suppported for current public key and diversifier on solomachine + case Misbehaviour: + h1 = misbehaviour.h1 + h2 = misbehaviour.h2 + pubkey = clientState.consensusState.publicKey + diversifier = clientState.consensusState.diversifier + timestamp = clientState.consensusState.timestamp + // assert that timestamp could have fooled the light client + assert(misbehaviour.h1.signature.timestamp >= timestamp) + assert(misbehaviour.h2.signature.timestamp >= timestamp) + // assert that signature data is different + assert(misbehaviour.h1.signature.data !== misbehaviour.h2.signature.data) + // assert that the signatures validate + assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h1.signature.data)) + assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h2.signature.data)) + } } ``` ### Misbehaviour predicate -Any duplicate signature on different messages by the current public key freezes a solo machine client. +Since misbehaviour is checked in `verifyClientMessage`, if the client message is of type `Misbehaviour` then we return true ```typescript -function checkMisbehaviourAndUpdateState( +function checkForMisbehaviour( clientState: ClientState, - misbehaviour: Misbehaviour) { - h1 = misbehaviour.h1 - h2 = misbehaviour.h2 - pubkey = clientState.consensusState.publicKey - diversifier = clientState.consensusState.diversifier - timestamp = clientState.consensusState.timestamp - // assert that timestamp could have fooled the light client - assert(misbehaviour.h1.signature.timestamp >= timestamp) - assert(misbehaviour.h2.signature.timestamp >= timestamp) - // assert that signature data is different - assert(misbehaviour.h1.signature.data !== misbehaviour.h2.signature.data) - // assert that the signatures validate - assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h1.signature.data)) - assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h2.signature.data)) - // freeze the client - clientState.frozen = true + clientMessage: ClientMessage) => bool { + switch typeof(ClientMessage) { + case Misbehaviour: + return true + } + return false } ``` -### State verification functions - -All solo machine client state verification functions simply check a signature, which must be provided by the solo machine. +### Update Functions -Note that value concatenation should be implemented in a state-machine-specific escaped fashion. +`UpdateState` updates the function for a regular update: ```typescript -function verifyClientState( +function updateState( clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - clientIdentifier: Identifier, - counterpartyClientState: ClientState) { - path = applyPrefix(prefix, "clients/{clientIdentifier}/clientState") - // ICS 003 will not increment the proof height after connection verification - // the solo machine client must increment the proof height to ensure it matches - // the expected sequence used in the signature - abortTransactionUnless(height + 1 == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + counterpartyClientState - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) + clientMessage: ClientMessage) { + clientState.consensusState.publicKey = header.newPublicKey + clientState.consensusState.diversifier = header.newDiversifier + clientState.consensusState.timestamp = header.timestamp clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp } +``` -function verifyClientConsensusState( - clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - clientIdentifier: Identifier, - consensusStateHeight: uint64, - consensusState: ConsensusState) { - path = applyPrefix(prefix, "clients/{clientIdentifier}/consensusState/{consensusStateHeight}") - // ICS 003 will not increment the proof height after connection or client state verification - // the solo machine client must increment the proof height by 2 to ensure it matches - // the expected sequence used in the signature - abortTransactionUnless(height + 2 == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + consensusState - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) - clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp -} +`UpdateStateOnMisbehaviour` updates the function after receving valid misbehaviour: -function verifyConnectionState( +```typescript +function updateStateOnMisbehaviour( clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - connectionIdentifier: Identifier, - connectionEnd: ConnectionEnd) { - path = applyPrefix(prefix, "connection/{connectionIdentifier}") - abortTransactionUnless(height == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + connectionEnd - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) - clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp + clientMessage: ClientMessage) { + // freeze the client + clientState.frozen = true } +``` -function verifyChannelState( - clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - portIdentifier: Identifier, - channelIdentifier: Identifier, - channelEnd: ChannelEnd) { - path = applyPrefix(prefix, "ports/{portIdentifier}/channels/{channelIdentifier}") - abortTransactionUnless(height == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + channelEnd - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) - clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp -} +### State verification functions -function verifyPacketData( - clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - portIdentifier: Identifier, - channelIdentifier: Identifier, - sequence: uint64, - data: bytes) { - path = applyPrefix(prefix, "ports/{portIdentifier}/channels/{channelIdentifier}/packets/{sequence}") - abortTransactionUnless(height == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + data - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) - clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp -} +All solo machine client state verification functions simply check a signature, which must be provided by the solo machine. -function verifyPacketAcknowledgement( - clientState: ClientState, - height: uint64, - prefix: CommitmentPrefix, - proof: CommitmentProof, - portIdentifier: Identifier, - channelIdentifier: Identifier, - sequence: uint64, - acknowledgement: bytes) { - path = applyPrefix(prefix, "ports/{portIdentifier}/channels/{channelIdentifier}/acknowledgements/{sequence}") - abortTransactionUnless(height == clientState.consensusState.sequence) - abortTransactionUnless(!clientState.frozen) - abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + acknowledgement - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) - clientState.consensusState.sequence++ - clientState.consensusState.timestamp = proof.timestamp -} +Note that value concatenation should be implemented in a state-machine-specific escaped fashion. -function verifyPacketReceiptAbsence( +```typescript +function verifyMembership( clientState: ClientState, height: uint64, - prefix: CommitmentPrefix, + // delayPeriod is unsupported on solomachines + // thus these fields are ignored + delayTimePeriod: uint64, + delayBlockPeriod: uint64, proof: CommitmentProof, - portIdentifier: Identifier, - channelIdentifier: Identifier, - sequence: uint64) { - path = applyPrefix(prefix, "ports/{portIdentifier}/channels/{channelIdentifier}/receipts/{sequence}") - abortTransactionUnless(height == clientState.consensusState.sequence) + path: CommitmentPath, + value: []byte) { + // the expected sequence used in the signature abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) + sigBytes = SignBytes( + Sequence: clientState.consensusState.sequence, + Timestamp: proof.timestamp, + Diversifier: clientState.consensusState.diversifier, + path: CommitmentPath.String(), + data: value, + ) + assert(checkSignature(clientState.consensusState.pubKey, sigBytes, proof.sig)) + + // increment sequence on each verification to provide + // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp } -function verifyNextSequenceRecv( +function verifyNonMembership( clientState: ClientState, height: uint64, - prefix: CommitmentPrefix, + // delayPeriod is unsupported on solomachines + // thus these fields are ignored + delayTimePeriod: uint64, + delayBlockPeriod: uint64, proof: CommitmentProof, - portIdentifier: Identifier, - channelIdentifier: Identifier, - nextSequenceRecv: uint64) { - path = applyPrefix(prefix, "ports/{portIdentifier}/channels/{channelIdentifier}/nextSequenceRecv") - abortTransactionUnless(height == clientState.consensusState.sequence) + path: CommitmentPath) { abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - value = clientState.consensusState.sequence + clientState.consensusState.diversifier + proof.timestamp + path + nextSequenceRecv + sigBytes = SignBytes( + Sequence: clientState.consensusState.sequence, + Timestamp: proof.timestamp, + Diversifier: clientState.consensusState.diversifier, + path: CommitmentPath.String(), + data: nil, + ) assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) + + // increment sequence on each verification to provide + // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp } @@ -353,6 +303,7 @@ None at present. December 9th, 2019 - Initial version December 17th, 2019 - Final first draft +August 15th, 2022 - Changes to align with 02-client-refactor in [\#813](https://github.com/cosmos/ibc/pull/813) ## Copyright From 005cb5fad583d8e0a08936051f7c45951440a497 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 16 Aug 2022 13:46:27 +0200 Subject: [PATCH 02/11] fix client message handler fns --- .../ics-006-solo-machine-client/README.md | 72 ++++++++++++++----- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index ae6f3c2e2..0883af46d 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -70,7 +70,6 @@ The `Height` of a solo machine is just a `uint64`, with the usual comparison ope ```typescript interface Header { - sequence: uint64 timestamp: uint64 signature: Signature newPublicKey: PublicKey @@ -153,26 +152,61 @@ function verifyClientMessage( clientMsg: ClientMessage) { switch typeof(ClientMessage) { case Header: - assert(header.sequence === clientState.consensusState.sequence) - assert(header.timestamp >= clientstate.consensusState.timestamp) - assert(checkSignature(header.newPublicKey, header.sequence, header.diversifier, header.signature)) + verifyHeader(clientState, clientMessage) // misbehaviour only suppported for current public key and diversifier on solomachine case Misbehaviour: - h1 = misbehaviour.h1 - h2 = misbehaviour.h2 - pubkey = clientState.consensusState.publicKey - diversifier = clientState.consensusState.diversifier - timestamp = clientState.consensusState.timestamp - // assert that timestamp could have fooled the light client - assert(misbehaviour.h1.signature.timestamp >= timestamp) - assert(misbehaviour.h2.signature.timestamp >= timestamp) - // assert that signature data is different - assert(misbehaviour.h1.signature.data !== misbehaviour.h2.signature.data) - // assert that the signatures validate - assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h1.signature.data)) - assert(checkSignature(pubkey, misbehaviour.sequence, diversifier, misbehaviour.h2.signature.data)) + } } + +function verifyHeader( + clientState: clientState, + clientMessage: clientMessage) { + assert(header.timestamp >= clientstate.consensusState.timestamp) + headerData = { + NewPublicKey: header.newPublicKey, + NewDiversifier: header.newDiversifier, + } + sigBytes = SignBytes( + Sequence: clientState.consensusState.sequence, + Timestamp: header.timestamp, + Diversifier: clientState.consensusState.diversifier, + Path: []byte{"SENTINEL_HEADER_PATH"} + Value: marshal(headerData) + ) + assert(checkSignature(cs.consensusState.publicKey, sigBytes, header.signature)) +} + +function verifyMisbehaviour( + clientState: clientState, + misbehaviour: Misbehaviour) { + s1 = misbehaviour.signatureOne + s2 = misbehaviour.signatureTwo + pubkey = clientState.consensusState.publicKey + diversifier = clientState.consensusState.diversifier + timestamp = clientState.consensusState.timestamp + // assert that timestamp could have fooled the light client + assert(misbehaviour.s1.timestamp >= timestamp) + assert(misbehaviour.s2.timestamp >= timestamp) + // assert that the signatures validate and that they are different + sigBytes1 = SignBytes( + Sequence: misbehaviour.sequence, + Timestamp: s1.timestamp, + Diversifier: cs.consensusState.diversifier, + Path: s1.path, + Data: s1.data + ) + sigBytes2 = SignBytes( + Sequence: misbehaviour.sequence, + Timestamp: s2.timestamp, + Diversifier: cs.consensusState.diversifier, + Path: s2.path, + Data: s2,.data + ) + assert(sigBytes1 != sigBytes2) + assert(checkSignature(pubkey, sigBytes1, clientState.consensusState.publicKey)) + assert(checkSignature(pubkey, sigBytes2, clientState.consensusState.publicKey)) +} ``` ### Misbehaviour predicate @@ -226,6 +260,8 @@ Note that value concatenation should be implemented in a state-machine-specific ```typescript function verifyMembership( clientState: ClientState, + // provided height is unnecessary for solomachine + // since clientState maintains the expected sequence height: uint64, // delayPeriod is unsupported on solomachines // thus these fields are ignored @@ -254,6 +290,8 @@ function verifyMembership( function verifyNonMembership( clientState: ClientState, + // provided height is unnecessary for solomachine + // since clientState maintains the expected sequence height: uint64, // delayPeriod is unsupported on solomachines // thus these fields are ignored From b8a55b0231e99da598a56ff8a7b0d1576bb162d9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 31 Oct 2022 14:26:48 +0100 Subject: [PATCH 03/11] address reviews --- .../ics-006-solo-machine-client/README.md | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 0883af46d..877d83c78 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -70,6 +70,7 @@ The `Height` of a solo machine is just a `uint64`, with the usual comparison ope ```typescript interface Header { + sequence: uint64 timestamp: uint64 signature: Signature newPublicKey: PublicKey @@ -77,6 +78,8 @@ interface Header { } ``` +`Header` implements the ClientMessage interface. + ### Signature Verification The solomachine public key must sign over the following struct: @@ -110,6 +113,8 @@ interface Misbehaviour { } ``` +`Misbehaviour` implements the ClientState interface. + ### Signatures Signatures are provided in the `Proof` field of client state verification functions. They include data & a timestamp, which must also be signed over. @@ -142,26 +147,26 @@ function latestClientHeight(clientState: ClientState): uint64 { } ``` +### ClientState Methods + +All of the functions defined below are methods on the `ClientState` interface. Thus, the solomachine clientstate is always in scope for these functions. + ### Validity predicate The solo machine client `verifyClientMessage` function checks that the currently registered public key and diversifier signed over the client message at the expected sequence. If the client message is an update, then it must be the current sequence. If the client message is misbehaviour then it must be the sequence of the misbehaviour. ```typescript -function verifyClientMessage( - clientState: ClientState, - clientMsg: ClientMessage) { +function verifyClientMessage(clientMsg: ClientMessage) { switch typeof(ClientMessage) { case Header: - verifyHeader(clientState, clientMessage) + verifyHeader(clientMessage) // misbehaviour only suppported for current public key and diversifier on solomachine case Misbehaviour: - + verifyMisbehaviour(clientMessage) } } -function verifyHeader( - clientState: clientState, - clientMessage: clientMessage) { +function verifyHeader(header: header) { assert(header.timestamp >= clientstate.consensusState.timestamp) headerData = { NewPublicKey: header.newPublicKey, @@ -177,9 +182,7 @@ function verifyHeader( assert(checkSignature(cs.consensusState.publicKey, sigBytes, header.signature)) } -function verifyMisbehaviour( - clientState: clientState, - misbehaviour: Misbehaviour) { +function verifyMisbehaviour(misbehaviour: Misbehaviour) { s1 = misbehaviour.signatureOne s2 = misbehaviour.signatureTwo pubkey = clientState.consensusState.publicKey @@ -214,9 +217,7 @@ function verifyMisbehaviour( Since misbehaviour is checked in `verifyClientMessage`, if the client message is of type `Misbehaviour` then we return true ```typescript -function checkForMisbehaviour( - clientState: ClientState, - clientMessage: ClientMessage) => bool { +function checkForMisbehaviour(clientMessage: ClientMessage) => bool { switch typeof(ClientMessage) { case Misbehaviour: return true @@ -230,9 +231,7 @@ function checkForMisbehaviour( `UpdateState` updates the function for a regular update: ```typescript -function updateState( - clientState: ClientState, - clientMessage: ClientMessage) { +function updateState(clientMessage: ClientMessage) { clientState.consensusState.publicKey = header.newPublicKey clientState.consensusState.diversifier = header.newDiversifier clientState.consensusState.timestamp = header.timestamp @@ -243,9 +242,7 @@ function updateState( `UpdateStateOnMisbehaviour` updates the function after receving valid misbehaviour: ```typescript -function updateStateOnMisbehaviour( - clientState: ClientState, - clientMessage: ClientMessage) { +function updateStateOnMisbehaviour(clientMessage: ClientMessage) { // freeze the client clientState.frozen = true } @@ -259,7 +256,6 @@ Note that value concatenation should be implemented in a state-machine-specific ```typescript function verifyMembership( - clientState: ClientState, // provided height is unnecessary for solomachine // since clientState maintains the expected sequence height: uint64, @@ -289,7 +285,6 @@ function verifyMembership( } function verifyNonMembership( - clientState: ClientState, // provided height is unnecessary for solomachine // since clientState maintains the expected sequence height: uint64, From 5c8ee6f1d7f52920b807f37203b1d180b21356cd Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 31 Oct 2022 14:56:02 +0100 Subject: [PATCH 04/11] continuted improvements --- .../ics-006-solo-machine-client/README.md | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 877d83c78..ff82de087 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -195,16 +195,16 @@ function verifyMisbehaviour(misbehaviour: Misbehaviour) { sigBytes1 = SignBytes( Sequence: misbehaviour.sequence, Timestamp: s1.timestamp, - Diversifier: cs.consensusState.diversifier, + Diversifier: diversifier, Path: s1.path, Data: s1.data ) sigBytes2 = SignBytes( Sequence: misbehaviour.sequence, Timestamp: s2.timestamp, - Diversifier: cs.consensusState.diversifier, + Diversifier: diversifier, Path: s2.path, - Data: s2,.data + Data: s2.data ) assert(sigBytes1 != sigBytes2) assert(checkSignature(pubkey, sigBytes1, clientState.consensusState.publicKey)) @@ -236,6 +236,7 @@ function updateState(clientMessage: ClientMessage) { clientState.consensusState.diversifier = header.newDiversifier clientState.consensusState.timestamp = header.timestamp clientState.consensusState.sequence++ + set("clients/{identifier}/clientState", clientState) } ``` @@ -245,6 +246,7 @@ function updateState(clientMessage: ClientMessage) { function updateStateOnMisbehaviour(clientMessage: ClientMessage) { // freeze the client clientState.frozen = true + set("clients/{identifier}/clientState", clientState) } ``` @@ -265,7 +267,7 @@ function verifyMembership( delayBlockPeriod: uint64, proof: CommitmentProof, path: CommitmentPath, - value: []byte) { + value: []byte): boolean { // the expected sequence used in the signature abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) @@ -273,15 +275,19 @@ function verifyMembership( Sequence: clientState.consensusState.sequence, Timestamp: proof.timestamp, Diversifier: clientState.consensusState.diversifier, - path: CommitmentPath.String(), + path: path.String(), data: value, ) - assert(checkSignature(clientState.consensusState.pubKey, sigBytes, proof.sig)) + proven = checkSignature(clientState.consensusState.pubKey, sigBytes, proof.sig) + if !proven { + return false + } // increment sequence on each verification to provide // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp + return true } function verifyNonMembership( @@ -293,22 +299,26 @@ function verifyNonMembership( delayTimePeriod: uint64, delayBlockPeriod: uint64, proof: CommitmentProof, - path: CommitmentPath) { + path: CommitmentPath): boolean { abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) sigBytes = SignBytes( Sequence: clientState.consensusState.sequence, Timestamp: proof.timestamp, Diversifier: clientState.consensusState.diversifier, - path: CommitmentPath.String(), + path: path.String(), data: nil, ) - assert(checkSignature(clientState.consensusState.pubKey, value, proof.sig)) + proven = checkSignature(clientState.consensusState.pubKey, value, proof.sig) + if !proven { + return false + } // increment sequence on each verification to provide // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp + return true } ``` From bbde565205d568a27387c1e0fcf61b61d0822d94 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 29 Nov 2022 13:34:55 -0500 Subject: [PATCH 05/11] Update spec/client/ics-006-solo-machine-client/README.md Co-authored-by: Damian Nolan --- spec/client/ics-006-solo-machine-client/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index ff82de087..f535156fe 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -176,7 +176,7 @@ function verifyHeader(header: header) { Sequence: clientState.consensusState.sequence, Timestamp: header.timestamp, Diversifier: clientState.consensusState.diversifier, - Path: []byte{"SENTINEL_HEADER_PATH"} + Path: []byte{"solomachine:header"} Value: marshal(headerData) ) assert(checkSignature(cs.consensusState.publicKey, sigBytes, header.signature)) From 92d5bef179079ba4dabf989a76e98a1a14e0989a Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 13 Dec 2022 18:25:52 -0500 Subject: [PATCH 06/11] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Carlos Rodriguez Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- .../ics-006-solo-machine-client/README.md | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index f535156fe..4b75ceaae 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -78,7 +78,7 @@ interface Header { } ``` -`Header` implements the ClientMessage interface. +`Header` implements the `ClientMessage` interface. ### Signature Verification @@ -101,9 +101,9 @@ interface SignBytes { ```typescript interface SignatureAndData { sig: Signature - path: Path + path: []byte data: []byte - timestamp: Timestamop + timestamp: Timestamp } interface Misbehaviour { @@ -113,7 +113,7 @@ interface Misbehaviour { } ``` -`Misbehaviour` implements the ClientState interface. +`Misbehaviour` implements the `ClientMessage` interface. ### Signatures @@ -149,14 +149,14 @@ function latestClientHeight(clientState: ClientState): uint64 { ### ClientState Methods -All of the functions defined below are methods on the `ClientState` interface. Thus, the solomachine clientstate is always in scope for these functions. +All of the functions defined below are methods on the `ClientState` interface. Thus, the solomachine client state is always in scope for these functions. ### Validity predicate The solo machine client `verifyClientMessage` function checks that the currently registered public key and diversifier signed over the client message at the expected sequence. If the client message is an update, then it must be the current sequence. If the client message is misbehaviour then it must be the sequence of the misbehaviour. ```typescript -function verifyClientMessage(clientMsg: ClientMessage) { +function verifyClientMessage(clientMsg: ClientMessage) { switch typeof(ClientMessage) { case Header: verifyHeader(clientMessage) @@ -172,14 +172,14 @@ function verifyHeader(header: header) { NewPublicKey: header.newPublicKey, NewDiversifier: header.newDiversifier, } - sigBytes = SignBytes( + signBytes = SignBytes( Sequence: clientState.consensusState.sequence, Timestamp: header.timestamp, Diversifier: clientState.consensusState.diversifier, Path: []byte{"solomachine:header"} Value: marshal(headerData) ) - assert(checkSignature(cs.consensusState.publicKey, sigBytes, header.signature)) + assert(checkSignature(cs.consensusState.publicKey, signBytes, header.signature)) } function verifyMisbehaviour(misbehaviour: Misbehaviour) { @@ -214,7 +214,7 @@ function verifyMisbehaviour(misbehaviour: Misbehaviour) { ### Misbehaviour predicate -Since misbehaviour is checked in `verifyClientMessage`, if the client message is of type `Misbehaviour` then we return true +Since misbehaviour is checked in `verifyClientMessage`, if the client message is of type `Misbehaviour` then we return true: ```typescript function checkForMisbehaviour(clientMessage: ClientMessage) => bool { @@ -271,14 +271,14 @@ function verifyMembership( // the expected sequence used in the signature abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - sigBytes = SignBytes( + signBytes = SignBytes( Sequence: clientState.consensusState.sequence, Timestamp: proof.timestamp, Diversifier: clientState.consensusState.diversifier, path: path.String(), data: value, ) - proven = checkSignature(clientState.consensusState.pubKey, sigBytes, proof.sig) + proven = checkSignature(clientState.consensusState.publicKey, signBytes, proof.sig) if !proven { return false } @@ -302,14 +302,14 @@ function verifyNonMembership( path: CommitmentPath): boolean { abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) - sigBytes = SignBytes( + signBytes = SignBytes( Sequence: clientState.consensusState.sequence, Timestamp: proof.timestamp, Diversifier: clientState.consensusState.diversifier, path: path.String(), data: nil, ) - proven = checkSignature(clientState.consensusState.pubKey, value, proof.sig) + proven = checkSignature(clientState.consensusState.publicKey, signBytes, proof.sig) if !proven { return false } From 1d71f8ecc808e9d4533a83a8ee9434136dacf51d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 13 Dec 2022 19:59:57 -0500 Subject: [PATCH 07/11] address colin review --- .../ics-006-solo-machine-client/README.md | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 4b75ceaae..0c01d6204 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -167,9 +167,10 @@ function verifyClientMessage(clientMsg: ClientMessage) { } function verifyHeader(header: header) { + assert(header.sequence === clientState.consensusState.sequence) assert(header.timestamp >= clientstate.consensusState.timestamp) headerData = { - NewPublicKey: header.newPublicKey, + NewPubKey: header.newPubKey, NewDiversifier: header.newDiversifier, } signBytes = SignBytes( @@ -188,9 +189,6 @@ function verifyMisbehaviour(misbehaviour: Misbehaviour) { pubkey = clientState.consensusState.publicKey diversifier = clientState.consensusState.diversifier timestamp = clientState.consensusState.timestamp - // assert that timestamp could have fooled the light client - assert(misbehaviour.s1.timestamp >= timestamp) - assert(misbehaviour.s2.timestamp >= timestamp) // assert that the signatures validate and that they are different sigBytes1 = SignBytes( Sequence: misbehaviour.sequence, @@ -206,9 +204,10 @@ function verifyMisbehaviour(misbehaviour: Misbehaviour) { Path: s2.path, Data: s2.data ) - assert(sigBytes1 != sigBytes2) - assert(checkSignature(pubkey, sigBytes1, clientState.consensusState.publicKey)) - assert(checkSignature(pubkey, sigBytes2, clientState.consensusState.publicKey)) + // either the path or data must be different in order for the misbehaviour to be valid + assert(s1.path != s2.path || s1.data != s2.data) + assert(checkSignature(pubkey, sigBytes1, misbehaviour.signatureOne.signature)) + assert(checkSignature(pubkey, sigBytes2, misbehaviour.signatureTwo.signature)) } ``` @@ -232,7 +231,7 @@ function checkForMisbehaviour(clientMessage: ClientMessage) => bool { ```typescript function updateState(clientMessage: ClientMessage) { - clientState.consensusState.publicKey = header.newPublicKey + clientState.consensusState.publicKey = header.newPubKey clientState.consensusState.diversifier = header.newDiversifier clientState.consensusState.timestamp = header.timestamp clientState.consensusState.sequence++ @@ -287,6 +286,11 @@ function verifyMembership( // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp + // unlike other clients, we must set the client state here because we + // mutate the clientState (increment sequence and set timestamp) + // thus the verification methods are stateful for the solomachine + // in order to prevent replay attacks + set("clients/{identifier}/clientState", clientState) return true } @@ -318,6 +322,11 @@ function verifyNonMembership( // replay protection clientState.consensusState.sequence++ clientState.consensusState.timestamp = proof.timestamp + // unlike other clients, we must set the client state here because we + // mutate the clientState (increment sequence and set timestamp) + // thus the verification methods are stateful for the solomachine + // in order to prevent replay attacks + set("clients/{identifier}/clientState", clientState) return true } ``` From 0f9eba1a224750fefd4fa79cca8b98e681a06b9b Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 13 Dec 2022 20:01:41 -0500 Subject: [PATCH 08/11] deprecate header sequence --- spec/client/ics-006-solo-machine-client/README.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 0c01d6204..dcbdc98c1 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -70,7 +70,7 @@ The `Height` of a solo machine is just a `uint64`, with the usual comparison ope ```typescript interface Header { - sequence: uint64 + sequence: uint64 // deprecated timestamp: uint64 signature: Signature newPublicKey: PublicKey @@ -167,7 +167,6 @@ function verifyClientMessage(clientMsg: ClientMessage) { } function verifyHeader(header: header) { - assert(header.sequence === clientState.consensusState.sequence) assert(header.timestamp >= clientstate.consensusState.timestamp) headerData = { NewPubKey: header.newPubKey, From cef04d0ef0ba3c9d007c36fe6c8e8e674fd69ac0 Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 14 Dec 2022 16:47:00 -0500 Subject: [PATCH 09/11] Apply suggestions from code review Co-authored-by: Damian Nolan --- spec/client/ics-006-solo-machine-client/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index dcbdc98c1..a53625a21 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -149,7 +149,7 @@ function latestClientHeight(clientState: ClientState): uint64 { ### ClientState Methods -All of the functions defined below are methods on the `ClientState` interface. Thus, the solomachine client state is always in scope for these functions. +All of the functions defined below are methods on the `ClientState` interface. Thus, the solo machine client state is always in scope for these functions. ### Validity predicate @@ -176,7 +176,7 @@ function verifyHeader(header: header) { Sequence: clientState.consensusState.sequence, Timestamp: header.timestamp, Diversifier: clientState.consensusState.diversifier, - Path: []byte{"solomachine:header"} + Path: []byte{"solomachine:header"}, Value: marshal(headerData) ) assert(checkSignature(cs.consensusState.publicKey, signBytes, header.signature)) @@ -226,7 +226,7 @@ function checkForMisbehaviour(clientMessage: ClientMessage) => bool { ### Update Functions -`UpdateState` updates the function for a regular update: +`UpdateState` updates the solo machine `ConsensusState` values using the provided client message header: ```typescript function updateState(clientMessage: ClientMessage) { From 7749fe7607bd480d45b5a284f84886470e6952fc Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 14 Dec 2022 17:19:42 -0500 Subject: [PATCH 10/11] address damian review --- spec/client/ics-006-solo-machine-client/README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index dcbdc98c1..a91d2e175 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -153,7 +153,7 @@ All of the functions defined below are methods on the `ClientState` interface. T ### Validity predicate -The solo machine client `verifyClientMessage` function checks that the currently registered public key and diversifier signed over the client message at the expected sequence. If the client message is an update, then it must be the current sequence. If the client message is misbehaviour then it must be the sequence of the misbehaviour. +The solo machine client `verifyClientMessage` function checks that the currently registered public key signed over the client message at the expected sequence with the current diversifier included in the client message. If the client message is an update, then it must be the current sequence. If the client message is misbehaviour then it must be the sequence of the misbehaviour. ```typescript function verifyClientMessage(clientMsg: ClientMessage) { @@ -252,8 +252,6 @@ function updateStateOnMisbehaviour(clientMessage: ClientMessage) { All solo machine client state verification functions simply check a signature, which must be provided by the solo machine. -Note that value concatenation should be implemented in a state-machine-specific escaped fashion. - ```typescript function verifyMembership( // provided height is unnecessary for solomachine From d6dc88a7924af97672e53db5634010e69c03f395 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sat, 17 Dec 2022 06:34:27 +0100 Subject: [PATCH 11/11] formatting nits --- .../ics-006-solo-machine-client/README.md | 112 +++++++++--------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/spec/client/ics-006-solo-machine-client/README.md b/spec/client/ics-006-solo-machine-client/README.md index 2ace289b6..144084cbd 100644 --- a/spec/client/ics-006-solo-machine-client/README.md +++ b/spec/client/ics-006-solo-machine-client/README.md @@ -167,46 +167,46 @@ function verifyClientMessage(clientMsg: ClientMessage) { } function verifyHeader(header: header) { - assert(header.timestamp >= clientstate.consensusState.timestamp) - headerData = { - NewPubKey: header.newPubKey, - NewDiversifier: header.newDiversifier, - } - signBytes = SignBytes( - Sequence: clientState.consensusState.sequence, - Timestamp: header.timestamp, - Diversifier: clientState.consensusState.diversifier, - Path: []byte{"solomachine:header"}, - Value: marshal(headerData) - ) - assert(checkSignature(cs.consensusState.publicKey, signBytes, header.signature)) + assert(header.timestamp >= clientstate.consensusState.timestamp) + headerData = { + newPubKey: header.newPubKey, + newDiversifier: header.newDiversifier, + } + signBytes = SignBytes( + sequence: clientState.consensusState.sequence, + timestamp: header.timestamp, + diversifier: clientState.consensusState.diversifier, + path: []byte{"solomachine:header"}, + value: marshal(headerData) + ) + assert(checkSignature(cs.consensusState.publicKey, signBytes, header.signature)) } function verifyMisbehaviour(misbehaviour: Misbehaviour) { - s1 = misbehaviour.signatureOne - s2 = misbehaviour.signatureTwo - pubkey = clientState.consensusState.publicKey - diversifier = clientState.consensusState.diversifier - timestamp = clientState.consensusState.timestamp - // assert that the signatures validate and that they are different - sigBytes1 = SignBytes( - Sequence: misbehaviour.sequence, - Timestamp: s1.timestamp, - Diversifier: diversifier, - Path: s1.path, - Data: s1.data - ) - sigBytes2 = SignBytes( - Sequence: misbehaviour.sequence, - Timestamp: s2.timestamp, - Diversifier: diversifier, - Path: s2.path, - Data: s2.data - ) - // either the path or data must be different in order for the misbehaviour to be valid - assert(s1.path != s2.path || s1.data != s2.data) - assert(checkSignature(pubkey, sigBytes1, misbehaviour.signatureOne.signature)) - assert(checkSignature(pubkey, sigBytes2, misbehaviour.signatureTwo.signature)) + s1 = misbehaviour.signatureOne + s2 = misbehaviour.signatureTwo + pubkey = clientState.consensusState.publicKey + diversifier = clientState.consensusState.diversifier + timestamp = clientState.consensusState.timestamp + // assert that the signatures validate and that they are different + sigBytes1 = SignBytes( + sequence: misbehaviour.sequence, + timestamp: s1.timestamp, + diversifier: diversifier, + path: s1.path, + data: s1.data + ) + sigBytes2 = SignBytes( + sequence: misbehaviour.sequence, + timestamp: s2.timestamp, + diversifier: diversifier, + path: s2.path, + data: s2.data + ) + // either the path or data must be different in order for the misbehaviour to be valid + assert(s1.path != s2.path || s1.data != s2.data) + assert(checkSignature(pubkey, sigBytes1, misbehaviour.signatureOne.signature)) + assert(checkSignature(pubkey, sigBytes2, misbehaviour.signatureTwo.signature)) } ``` @@ -216,11 +216,11 @@ Since misbehaviour is checked in `verifyClientMessage`, if the client message is ```typescript function checkForMisbehaviour(clientMessage: ClientMessage) => bool { - switch typeof(ClientMessage) { - case Misbehaviour: - return true - } - return false + switch typeof(ClientMessage) { + case Misbehaviour: + return true + } + return false } ``` @@ -230,11 +230,11 @@ function checkForMisbehaviour(clientMessage: ClientMessage) => bool { ```typescript function updateState(clientMessage: ClientMessage) { - clientState.consensusState.publicKey = header.newPubKey - clientState.consensusState.diversifier = header.newDiversifier - clientState.consensusState.timestamp = header.timestamp - clientState.consensusState.sequence++ - set("clients/{identifier}/clientState", clientState) + clientState.consensusState.publicKey = header.newPubKey + clientState.consensusState.diversifier = header.newDiversifier + clientState.consensusState.timestamp = header.timestamp + clientState.consensusState.sequence++ + set("clients/{identifier}/clientState", clientState) } ``` @@ -242,9 +242,9 @@ function updateState(clientMessage: ClientMessage) { ```typescript function updateStateOnMisbehaviour(clientMessage: ClientMessage) { - // freeze the client - clientState.frozen = true - set("clients/{identifier}/clientState", clientState) + // freeze the client + clientState.frozen = true + set("clients/{identifier}/clientState", clientState) } ``` @@ -268,9 +268,9 @@ function verifyMembership( abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) signBytes = SignBytes( - Sequence: clientState.consensusState.sequence, - Timestamp: proof.timestamp, - Diversifier: clientState.consensusState.diversifier, + sequence: clientState.consensusState.sequence, + timestamp: proof.timestamp, + diversifier: clientState.consensusState.diversifier, path: path.String(), data: value, ) @@ -304,9 +304,9 @@ function verifyNonMembership( abortTransactionUnless(!clientState.frozen) abortTransactionUnless(proof.timestamp >= clientState.consensusState.timestamp) signBytes = SignBytes( - Sequence: clientState.consensusState.sequence, - Timestamp: proof.timestamp, - Diversifier: clientState.consensusState.diversifier, + sequence: clientState.consensusState.sequence, + timestamp: proof.timestamp, + diversifier: clientState.consensusState.diversifier, path: path.String(), data: nil, )