v3: Change SDK::send's params to take new type IpldBlock #1152
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 tasks
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1152 +/- ##
===========================================
- Coverage 43.10% 25.69% -17.41%
===========================================
Files 128 96 -32
Lines 13110 7767 -5343
===========================================
- Hits 5651 1996 -3655
+ Misses 7459 5771 -1688
|
Stebalien
reviewed
Nov 28, 2022
shared/src/ipld_block.rs
Outdated
Comment on lines
20
to
22
| // IPLD_RAW => BytesDeserializer::new(self.data.as_slice()) | ||
| // .deser() | ||
| // .map_err(Into::into), |
There was a problem hiding this comment.
Does this not work? (any reason not to just enable it?)
shared/src/ipld_block.rs
Outdated
| use fvm_ipld_encoding::DAG_CBOR; | ||
| use {serde, serde_ipld_dagcbor}; | ||
|
|
||
| // TODO: Slapped the Serialize derivations on for some actors testing, not clear to me it should stay |
There was a problem hiding this comment.
Yep, they shouldn't. Otherwise, we'll serialize twice.
arajasek
force-pushed
the
asr/ipld-block
branch
2 times, most recently
from
efeb3f4 to
bfab2e4
Compare
arajasek
force-pushed
the
asr/ipld-block
branch
3 times, most recently
from
105786f to
a31c967
Compare
anorth
approved these changes
Dec 1, 2022
arajasek
force-pushed
the
asr/ipld-block
branch
2 times, most recently
from
b2ce904 to
ecdb0ec
Compare
arajasek
changed the title
arajasek
force-pushed
the
asr/ipld-block
branch
4 times, most recently
from
cf4594e to
8dcfe70
Compare
Stebalien
reviewed
Dec 5, 2022
| @@ -135,7 +139,10 @@ fn invoke_method(blk: u32, method: u64) -> u32 { | |||
| let output = sdk::send::send( | |||
| &Address::new_id(sdk::message::receiver()), | |||
| 4, | |||
| RawBytes::new("input".into()), | |||
| Some(IpldBlock { | |||
| codec: DAG_CBOR, | |||
There was a problem hiding this comment.
nit: now that we can, consider using DAG_RAW? (if it works)
Stebalien
approved these changes
Dec 5, 2022
arajasek
force-pushed
the
asr/ipld-block
branch
from
8dcfe70 to
f059465
Compare
IpldBlock supports multiple codecs, including IPLD_RAW. Serializing byte vectors will correctly serialize anything that serializes to "raw bytes". However, it _won't_ serialize "byte lists", so `Vec<u8>` and `[u8]` won't work. But that shouldn't be an issue in practice. Users _should_ serialize to/from `ByteBuf`.
arajasek
force-pushed
the
asr/ipld-block
branch
from
f059465 to
a8f0d84
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Towards #987, but doesn't close it.
Introduces a new IpldBlock type that can support various codecs (currently DAG_CBOR and IPLD_RAW), and switches the SDK::send method to use
Option<IpldBlock>for params.