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

Simplify solo machine SignBytes construction #1141

Closed
6 tasks
colin-axner opened this issue Mar 17, 2022 · 1 comment
Closed
6 tasks

Simplify solo machine SignBytes construction #1141

colin-axner opened this issue Mar 17, 2022 · 1 comment

Comments

@colin-axner
Copy link
Contributor

Summary

The solo machine has unnecessary layering when constructing the signature bytes the solo machine will sign over. The layers were added via a misunderstanding of the security implications. The current construction does not support the proposed changes in the spec repo to support Generic Verification functions. We should simplify the solo machine signature bytes to allow for the solo machine to verify signatures over new paths without modifying the existing code.

Problem Definition

The solo machine currently requires each path verified to have an associated DataType. Adding a type requires adding a new data type. This was added via a misunderstanding of the security implications. It was noted that the proto definitions do not provide uniqueness and we want to ensure that a signature corresponds to a specific path. What was missed is that the uniqueness is not provided by the proto definition, but by the usage of the proto definition. The path provided by core IBC will be unique and is already encoded into the signature data.

Proposal

Backward compatibility considerations

Doing these changes will change the verification of solo machine signatures. That is, solo machines would need to correctly construct their signature data based on the version of 06-solomachine the chain they are talking to is using.

I think the practical approach is to deprecate the existing functionality and support it as legacy handling for a certain amount of time (giving chains/solo machines time to update to the newest changes). We could either try to construct the new and old sign bytes and see if one of them verifies or create a temporary requirement that the diversifier append a -06-solomachine-version-1 to indicate it will use the new functionality.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner changed the title Simplify solo machine Simplify solo machine SignBytes construction Mar 17, 2022
@crodriguezvega crodriguezvega moved this to Todo in ibc-go Mar 21, 2022
@crodriguezvega crodriguezvega added the needs discussion Issues that need discussion before they can be worked on label Mar 21, 2022
@crodriguezvega crodriguezvega self-assigned this Mar 21, 2022
@crodriguezvega crodriguezvega moved this from Todo to Backlog in ibc-go May 1, 2022
@crodriguezvega crodriguezvega removed their assignment Jul 10, 2022
@damiannolan damiannolan self-assigned this Jul 11, 2022
@crodriguezvega crodriguezvega moved this from Backlog to Todo in ibc-go Jul 11, 2022
@crodriguezvega crodriguezvega moved this from Todo to In progress in ibc-go Jul 11, 2022
@damiannolan damiannolan moved this from In progress to In review in ibc-go Jul 19, 2022
@crodriguezvega crodriguezvega removed the needs discussion Issues that need discussion before they can be worked on label Jul 22, 2022
@crodriguezvega
Copy link
Contributor

Closed by #1687

Repository owner moved this from In review to Done in ibc-go Aug 4, 2022
CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

Closes: cosmos#1107 

<!-- 
Please provide an explanation of the PR, including the appropriate
context,
background, goal, and rationale. If there is an issue with this
information,
please provide a tl;dr and link the issue. 
-->

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

3 participants