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

Fix Asset.UnmarshallBinary not working #36

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

DragonDev1906
Copy link
Contributor

  • The method did not have a pointer receiver and thus could not set the value, resulting in the previous (default) value instead of the just unmarshalled one.
  • Additionally, the order was different from Asset.MarshallBinary, which meant unmarshalling a value just marshalled would result in the wrong value or an error.

The added test coveres both these cases.

Signed-off-by: Jens Winkle [email protected]

RmbRT
RmbRT previously approved these changes Jan 6, 2023
- The method did not have a pointer receiver and thus could not set the
  value, resulting in the previous (default) value instead of the just
  unmarshalled one.
- Additionally, the order was different from `Asset.MarshallBinary`,
  which meant unmarshalling a value just marshalled would result in
  the wrong value or an error.

The added test coveres both these cases.

Signed-off-by: Jens Winkle <[email protected]>
@RmbRT RmbRT merged commit 5bbe33a into hyperledger-labs:main Jan 6, 2023
@DragonDev1906 DragonDev1906 deleted the asset-unmarshal branch January 7, 2023 17:16
DragonDev1906 added a commit to hyperledger-labs/perun-rs that referenced this pull request Mar 14, 2023
- Go: Change go-side to accept any incomming channel (and a wallet
  related fix)
- Rust: Uncomment channel proposal and receive of accept message
- Rust: Change order of values in encoded assets (Asset.MarshalBinary),
  see hyperledger-labs/perun-eth-backend#36 for
  why this wasn't correct beforehand. Additionally, this PR is required
  for this example to work correctly, as go-perun will throw away all
  proposed channels because the Asset list isn't
  deserialized/unmarshaled correctly.

Signed-off-by: Jens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants