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

IPLD Block Abstraction #758

Closed
Stebalien opened this issue Oct 17, 2022 · 0 comments · Fixed by #913
Closed

IPLD Block Abstraction #758

Stebalien opened this issue Oct 17, 2022 · 0 comments · Fixed by #913
Assignees
Labels

Comments

@Stebalien
Copy link
Member

Stebalien commented Oct 17, 2022

Currently, the actor's runtime passes around RawBytes which are assumed to be CBOR (or "nothing" if empty). However:

  1. We'd like to better support IPLD_RAW parameters (e.g., in the EVM). I.e., not DAG_CBOR.
  2. We should explicitly support a more "typed" nothing. Currently, we use an "empty" RawBytes for this, but that's not actually valid CBOR.

Proposal: Introduce a new IpldBlock type and change the runtime/actor methods to use Option<IpldBlock> in parameters and return values (instead of RawBytes).

pub struct IpldBlock {
    pub codec: u64,
    pub data: Vec<u8>,
}

impl IpldBlock {
    pub fn deserialize<'de, T>(&'de self) -> Result<T, ActorError>
    where
        T: Deserialize<'de>,
    {
        match self.codec {
            IPLD_RAW => todo!("deserialize as 'bytes' only"),
            DAG_CBOR => todo!("deserialize into arbitrary types as cbor"),
            _ => Err(actor_error!(serialization; "unsupported codec {}", self.codec)),
        }
    }
    pub fn serialize<T: Serialize>(codec: u64, value: &T) -> Result<Self, ActorError> {
        // Attempt to encode T with the specified codec.
        // ...
    }
    pub fn serialize_cbor<T: Serialize>(codec: u64, value: &T) -> Result<Self, ActorError> {
        self.serialize(DAG_CBOR, value)
    }
}

To avoid having to make too many manual changes up front, I'm proposing the following "tooling assisted" upgrade path:

  1. Mass rename Runtime::send to Runtime::send_cbor.
  2. Mass rename ActorCode::invoke_method to ActorCode::invoke_method_cbor.
  3. Introduce a new Runtime::send, making Runtime::send_cbor a special case that only handles cbor.
  4. Introduce a new ActorCode::invoke_method call that delegates to ActorCode::invoke_method_cbor by default.

This will create a fairly large diff, but shouldn't require too much manual work.

Stebalien added a commit that referenced this issue Oct 18, 2022
The previous code was passing parameters as raw bytes, but telling the
system that these bytes were valid CBOR. Unfortunately:

- This is incorrect and will break once we actually start enforcing this.
- This makes it _very_ difficult to test the EVM because we have quite a
  bit of testing code that assumes CBOR.

The long-term solution is to correctly handle IPLD (see
#758). But the
short-term solution is to just encode/decode parameters as CBOR.
Stebalien added a commit that referenced this issue Oct 18, 2022
The previous code was passing parameters as raw bytes, but telling the
system that these bytes were valid CBOR. Unfortunately:

- This is incorrect and will break once we actually start enforcing this.
- This makes it _very_ difficult to test the EVM because we have quite a
  bit of testing code that assumes CBOR.

The long-term solution is to correctly handle IPLD (see
#758). But the
short-term solution is to just encode/decode parameters as CBOR.
Stebalien added a commit that referenced this issue Oct 19, 2022
* fix: encode/decode EVM parameters as CBOR

The previous code was passing parameters as raw bytes, but telling the
system that these bytes were valid CBOR. Unfortunately:

- This is incorrect and will break once we actually start enforcing this.
- This makes it _very_ difficult to test the EVM because we have quite a
  bit of testing code that assumes CBOR.

The long-term solution is to correctly handle IPLD (see
#758). But the
short-term solution is to just encode/decode parameters as CBOR.

* fix: only use RawBytes for CBOR

"RawBytes" is for storing serialized CBOR (per the docs). Any other uses
are bugs.
@jennijuju jennijuju added the P2 label Nov 25, 2022
@jennijuju jennijuju moved this to 📋 Backlog in Network v18 Nov 25, 2022
@arajasek arajasek self-assigned this Nov 27, 2022
@jennijuju jennijuju moved this from 📋 Nice To Haves to 🏗 In progress in Network v18 Dec 7, 2022
@jennijuju jennijuju linked a pull request Dec 13, 2022 that will close this issue
Repository owner moved this from 🏗 In progress to ✅ Done in Network v18 Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants