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

The Address type is a compatibility hazard #738

Open
Stebalien opened this issue Oct 6, 2022 · 5 comments
Open

The Address type is a compatibility hazard #738

Stebalien opened this issue Oct 6, 2022 · 5 comments
Milestone

Comments

@Stebalien
Copy link
Member

Stebalien commented Oct 6, 2022

Currently, the Address type validates the underlying address format and understands all the different protocols. Unfortunately, this is a compatibility hazard that could make it difficult to add new address types to the system.

E.g.:

  • User deploy actors, depending on an SDK that assumes that all addresses will fit within some limited set of address types.
  • We add an address type.
  • Some syscalls, etc. may now panic, ignore the address, etc.

One solution would be to make the address type care less about the actual address format (potentially adding a "validate" method to be called where absolutely necessary).

@Stebalien Stebalien added this to the M2.2 milestone Oct 6, 2022
@anorth
Copy link
Member

anorth commented Oct 6, 2022

100%. See also: Signature.

@mriise
Copy link
Contributor

mriise commented Oct 18, 2022

I did some playing around and came up with this: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=0191e82bcb54280bba0f0fcb03d13003

I really like how to::<T> ended up, so we can assert that we are reading the expected type from the generic address. That paired with concrete types of each address means there will be a lot less addr.id().unwrap() and a lot less cases where you see an Address and wonder "wtf is this expected to be"

I'm not fully set on the implementatoin of GenericAddress, i think there can be some improvement such as using an inlined vec, but its better than using a buf of the fully encoded size. I also havent decided how to expose a "validate is one of these type" sort of thing, but i suppose it can just be return the type enum and let the caller do a match.

@Stebalien
Copy link
Member Author

How does this solve the problem stated here? Basically, what we need here is a type that can deal with "unknown" address classes.

@mriise
Copy link
Contributor

mriise commented Oct 18, 2022

AddressType is explicitly #[non_exaustive] so any attempt at matching the enum must have a wildcard handle unknown variants. GenericAddress in this case could hold the unknown address class. Though I would like to have some explicit size limit to the container inside GenericAddress so we can use inline buff instead of a vec and having a fully class would mean unknown encoded size, which would mean we would need to do something like smallvec that spills to heap.

@Stebalien
Copy link
Member Author

The real concern here is that existing actors (per-compiled) might start rejecting addresses from the system (if the system introduces new addresses).

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

No branches or pull requests

3 participants