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

Protocol message versioning #1140

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ At the moment this project **does not** adhere to
was added to the staking extension pallet's `Config` trait.
- In [#1134](https://github.com/entropyxyz/entropy-core/pull/1134/) the ```no-sync``` option was removed

### Added
- Protocol message versioning ([#1140](https://github.com/entropyxyz/entropy-core/pull/1140))

### Changed
- Use correct key rotation endpoint in OCW ([#1104](https://github.com/entropyxyz/entropy-core/pull/1104))
- Change attestation flow to be pull based ([#1109](https://github.com/entropyxyz/entropy-core/pull/1109/))
Expand Down
7 changes: 7 additions & 0 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,13 @@ use synedrion::{
AuxInfo, ThresholdKeyShare,
};

/// The current version number of the protocol message format or protocols themselves
pub const PROTOCOL_MESSAGE_VERSION: u32 = 1;

/// Currently supported protocol message versions for backward compatibility
/// This must contain the current version
pub const SUPPORTED_PROTOCOL_MESSAGE_VERSIONS: [u32; 1] = [PROTOCOL_MESSAGE_VERSION];

/// Identifies a party participating in a protocol session
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)]
pub struct PartyId(AccountId32);
Expand Down
9 changes: 9 additions & 0 deletions crates/protocol/src/protocol_transport/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,12 @@ pub enum EncryptedConnectionErr {
#[error("Could not get remote public key")]
RemotePublicKey,
}

/// Error when checking supported protocol versions
#[derive(Debug, Error, PartialEq)]
pub enum ProtocolVersionMismatchError {
#[error("This version of the protocol is newer than ours - we are on version {0}")]
VersionTooNew(u32),
#[error("This version of the protocol is no longer supported - the oldest we support is {0}")]
VersionTooOld(u32),
}
59 changes: 56 additions & 3 deletions crates/protocol/src/protocol_transport/subscribe_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use crate::SessionId;
use crate::{SessionId, PROTOCOL_MESSAGE_VERSION, SUPPORTED_PROTOCOL_MESSAGE_VERSIONS};
use serde::{Deserialize, Serialize};
use sp_core::{sr25519, Pair};
use subxt::utils::AccountId32;

use super::errors::ProtocolVersionMismatchError;

/// A message sent by a party when initiating a websocket connection to participate
/// in the signing or DKG protcol
#[derive(Debug, Clone, Serialize, Deserialize)]
Expand All @@ -29,14 +31,20 @@ pub struct SubscribeMessage {
pub public_key: sr25519::Public,
/// Signature to authenticate connecting party
pub signature: sr25519::Signature,
/// Specifies the version of the protocol messages which will be used for this session
pub version: u32,
}

impl SubscribeMessage {
pub fn new(session_id: SessionId, pair: &sr25519::Pair) -> Result<Self, bincode::Error> {
let session_id_serialized = bincode::serialize(&session_id)?;

let signature = pair.sign(&session_id_serialized);
Ok(Self { session_id, public_key: pair.public(), signature })
Ok(Self {
session_id,
public_key: pair.public(),
signature,
version: PROTOCOL_MESSAGE_VERSION,
})
}

pub fn account_id(&self) -> AccountId32 {
Expand All @@ -47,4 +55,49 @@ impl SubscribeMessage {
let session_id_serialized = bincode::serialize(&self.session_id)?;
Ok(sr25519::Pair::verify(&self.signature, session_id_serialized, &self.public_key))
}

pub fn check_supported(&self) -> Result<(), ProtocolVersionMismatchError> {
if self.version > PROTOCOL_MESSAGE_VERSION {
Err(ProtocolVersionMismatchError::VersionTooNew(PROTOCOL_MESSAGE_VERSION))
} else if !SUPPORTED_PROTOCOL_MESSAGE_VERSIONS.contains(&self.version) {
Err(ProtocolVersionMismatchError::VersionTooOld(
*SUPPORTED_PROTOCOL_MESSAGE_VERSIONS
.iter()
.min()
.expect("At least one protocol message version must be supported"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think expect is ok here because it will only fail if SUPPORTED_PROTOCOL_MESSAGE_VERSIONS, which is a const, is empty.

))
} else {
Ok(())
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_protocol_version_check() {
let session_id = SessionId::Dkg { block_number: 0 };
let pair = sr25519::Pair::from_seed(&[0; 32]);
let subscribe_message = SubscribeMessage::new(session_id.clone(), &pair).unwrap();
assert!(subscribe_message.check_supported().is_ok());

let session_id_serialized = bincode::serialize(&session_id).unwrap();
let signature = pair.sign(&session_id_serialized);
let mut subscribe_message =
SubscribeMessage { session_id, public_key: pair.public(), signature, version: 0 };
assert_eq!(
subscribe_message.check_supported(),
Err(ProtocolVersionMismatchError::VersionTooOld(
*SUPPORTED_PROTOCOL_MESSAGE_VERSIONS.iter().min().unwrap()
))
);

subscribe_message.version = 2;
assert_eq!(
subscribe_message.check_supported(),
Err(ProtocolVersionMismatchError::VersionTooNew(PROTOCOL_MESSAGE_VERSION))
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ pub enum SubscribeErr {
UserError(String),
#[error("Listener: {0}")]
Listener(#[from] entropy_protocol::errors::ListenerErr),
#[error("Protocol version mismatch: {0}")]
VersionMismatch(
#[from] entropy_protocol::protocol_transport::errors::ProtocolVersionMismatchError,
),
}

impl IntoResponse for SubscribeErr {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub async fn open_protocol_connections(
.map_err(|e| ProtocolErr::EncryptedConnection(e.to_string()))?;
let subscribe_response: Result<(), String> = bincode::deserialize(&response_message)?;
if let Err(error_message) = subscribe_response {
// In future versions, we can check here if the error is VersionTooNew(version)
// and if possible the downgrade protocol messages used to be backward compatible
return Err(ProtocolErr::BadSubscribeMessage(error_message));
}

Expand Down Expand Up @@ -158,6 +160,10 @@ async fn handle_initial_incoming_ws_message(
let msg: SubscribeMessage = bincode::deserialize(&serialized_subscribe_message)?;
tracing::info!("Got ws connection, with message: {msg:?}");

// In future versions we may have backwards compatibility with the
// old version and be able to recover here
msg.check_supported()?;

if !msg.verify()? {
return Err(SubscribeErr::InvalidSignature("Invalid signature."));
}
Expand Down
Loading