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

Add an abstract type for wallet.Sig #292

Open
manoranjith opened this issue Dec 13, 2021 · 2 comments
Open

Add an abstract type for wallet.Sig #292

manoranjith opened this issue Dec 13, 2021 · 2 comments
Labels
duplicate This issue or pull request already exists

Comments

@manoranjith
Copy link
Contributor

manoranjith commented Dec 13, 2021

Location

[wallet]

Problem

Currently wallet.Sig is defined as a variable size byte array in the abstract wallet package. However, the length of the signature depends upon the signature scheme that we use, which in-turn depends upon the wallet backend we use. Hence, in order to Encode/Decode a wallet.Sig, we directly use the wallet.Sig.Encode / wallet.Backend.DecodeSig` functions.

The problem is that, these Encode/DecodeSig functions do two different things:

1. Convert the blockchain specific concrete data to/from binary format (byte array).
2. Write/Read the data in binary format to/from the wire.

Proposal

As we did for Address, Action, Data and other types (see #257), we can change the definition of wallet.Sig to binary.Unmarshaler and binary.Marshaler. Then

  1. The wallet backend can implement the MarshalBinary, UnmarshalBinary functions. that convert these types to/from binary format.
  2. These type can then be directly decoded using perunio.(En|De)code functions.

Another point to be noted

This will also, remove the usage of perunio package in the channel and wallet packages in backend/ethereum. Using perunio package in these places is a concern because, perunio will now be treated as a specific implementation of the wire protocol and these packages should not directly depend on a specific implementation of wire protocol. Also, since we have introduced (Un)marshalBinary type of each of the abstract types like Action, Address, Data, App (see #257) and Sig (this issue), these functions can be used to convert these types to/from their binary representation.

@manoranjith
Copy link
Contributor Author

@matthiasgeihs

@matthiasgeihs
Copy link
Contributor

Relates to / duplicate of #168

@cryptphil cryptphil added the duplicate This issue or pull request already exists label Sep 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants