Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add borsh derive for MsgTransfer #845

Merged
merged 2 commits into from
Aug 30, 2023
Merged

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Aug 30, 2023

No description provided.

@plafer plafer requested a review from Farhad-Shabani August 30, 2023 15:24
@plafer plafer marked this pull request as ready for review August 30, 2023 15:25
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Patch coverage: 81.96% and project coverage change: +0.08% 🎉

Comparison is base (56e7e62) 71.47% compared to head (a320cd8) 71.55%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #845      +/-   ##
==========================================
+ Coverage   71.47%   71.55%   +0.08%     
==========================================
  Files         124      124              
  Lines       14983    15044      +61     
==========================================
+ Hits        10709    10765      +56     
- Misses       4274     4279       +5     
Files Changed Coverage Δ
crates/ibc/src/applications/transfer/coin.rs 80.00% <0.00%> (-1.09%) ⬇️
crates/ibc/src/applications/transfer/denom.rs 74.32% <0.00%> (-1.16%) ⬇️
...tes/ibc/src/applications/transfer/msgs/transfer.rs 35.36% <0.00%> (-0.44%) ⬇️
crates/ibc/src/applications/transfer/packet.rs 95.08% <0.00%> (-1.59%) ⬇️
crates/ibc/src/applications/transfer/amount.rs 85.26% <92.59%> (+24.28%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment on lines +55 to +60
if bytes_read != 32 {
return Err(borsh::maybestd::io::Error::new(
borsh::maybestd::io::ErrorKind::InvalidInput,
format!("Expected to read 32 bytes, read {bytes_read}"),
));
}
Copy link
Member

Choose a reason for hiding this comment

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

Since the `buf`` is of fixed 32 size, would this error ever occur?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, if somehow we read less than 32 bytes

@Farhad-Shabani Farhad-Shabani merged commit 1833df5 into main Aug 30, 2023
@Farhad-Shabani Farhad-Shabani deleted the plafer/borsh-msg-transfer branch August 30, 2023 21:09
@Farhad-Shabani Farhad-Shabani added this to the v0.45.0 milestone Sep 7, 2023
Farhad-Shabani pushed a commit that referenced this pull request Sep 9, 2024
* add borsh derive for `MsgTransfer`

* changelog
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants