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

Standardize solo machine signature format in proto definitions #7171

Closed
2 of 4 tasks
colin-axner opened this issue Aug 26, 2020 · 4 comments · Fixed by #7237
Closed
2 of 4 tasks

Standardize solo machine signature format in proto definitions #7171

colin-axner opened this issue Aug 26, 2020 · 4 comments · Fixed by #7237
Assignees

Comments

@colin-axner
Copy link
Contributor

colin-axner commented Aug 26, 2020

Summary

As mentioned during the pr review for solo machine, the signature format should use a standardized proto definition instead of concatenating byte string.

Proposal

Create proto definitions for each type of signature needed to verify counterparty state.

Some questions I still have:

  • do headers need timestamps?
  • do we want to allow extra data to be included in the signature (an additional []byte field to each definition)?
message SignBytes {
  uint64 sequence = 1;
  bytes data = 2; // proto marshaled definitions below
}

message HeaderData {
  google.protobuf.Any new_pub_key = 2;
}

message ClientStateData {
  uint64 timestamp = 1;
  bytes path = 2;
  google.protobuf.Any client_state = 3; 
}

message ConsensusStateData {
  uint64 timestamp = 1;
  bytes path = 2;
  google.protobuf.Any consensus_state = 3; 
}

message ConnectionStateData {
  uint64 timestamp = 1;
  bytes path = 2;
  ibc.connection.ConnectionEnd connection = 3; 
}

message ChannelStateData {
  uint64 timestamp = 1;
  bytes path = 2;
  ibc.channel.Channel  channel = 3; 
}

message PacketCommitmentData {
  uint64 timestamp = 1;
  bytes path = 2;
  bytes commitment = 3; 
}

message PacketAcknowledgementData {
  uint64 timestamp = 1;
  bytes path = 2;
  bytes acknowledgement = 3; 
}

message PacketAcknowledgementAbsenseData {
  uint64 timestamp = 1;
  bytes path = 2;
}

message NextSequenceRecvData {
  uint64 timestamp = 1;
  bytes path = 2;
  uint64 next_seq_recv = 3;
}

Because we need to know the sequence signed to prove misbehaviour, I added the SignBytes layer to encapsulate the non-sequence signed data.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@cwgoes cwgoes added this to the IBC 1.0 milestone Aug 26, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Aug 26, 2020

do headers need timestamps?

They can have timestamps, at minimum we should not reset the timestamp to zero on a header update.

A client's timestamp should never decrease.

do we want to allow extra data to be included in the signature (an additional []byte field to each definition)?

I don't think this is necessary but cc @warner

@colin-axner colin-axner self-assigned this Aug 27, 2020
@fedekunze
Copy link
Collaborator

I can work on this if you want @colin-axner

@colin-axner
Copy link
Contributor Author

@fedekunze go for it 👍

@colin-axner colin-axner assigned fedekunze and unassigned colin-axner Sep 2, 2020
@cwgoes
Copy link
Contributor

cwgoes commented Sep 2, 2020

Also, perhaps at the same time, we should add the signature diversifier per cosmos/ibc#476.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants