Encapsulate the distinction between SC address and User address.

[Ben-PH]

Is your feature request related to a problem? Please describe.

At the moment, an address is any address, but there are differences when using it depending on context.

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

[Ben-PH]

@damip I was given a short brief on this by @AurelienFT.

In your own time, could you please fill out the issue in more detail. I am prioritising #3275 for for now, at least until I can grok this issue better.

[damip]

One idea would be to extend what I explained here: #3417 (comment) with extra entries in the case of the address enum:

enum Address {
    EOA_V1(EOA_V1_Address),
    EOA_V2(EOA_V2_Address),
    SC_V1(SC_V1_Address),
    SC_V2(SC_V2_Address)
}

impl Address {
    pub fn is_eoa(&self) -> bool { ... }  // returns true if the address is externally owned (derived from a pubkey)
}

in the example above, we have 4 address types:

• two versions of externally owned account addresses
• two versions of smart contract account addresses

[Ben-PH]

For the time being, i'm going to make things naive to versioning, but here's the idea right now.

enum CtxAddress {
   User(UsrAddress), // wraps `Hash`
   SC(SCAddress),    // wraps slot, idx, read-only.

For use contexts where, for example, it would be an error to pass in a user address:

fn set_bytecode(..., addr: CtxAddress) -> Result<...> {
   let CtxAddress::SC(addr) = addr else {
       // TODO: raise the error at compile time
       return Err(...);
   }
   // ...
}

The idea being that in the future, we could have something like this for the general case.

fn addr_already_exists(&self, addr: impl Address) -> bool;
fn set_bytecode(..., addr: SCAddress)  -> ...;

Using the enum approach before diving into using the Address trait allows us to better understand how valuable a trait approach might actually be, and how it might best be used.

The question remains, will we be wanting to have versioning for an SC addr, and User addr seperately? or do we want it global on the enum? That's not something that

[AurelienFT]

For the versioning side we will go for having a pair of two varint as a version in Address. One version for the enum CtxAddress and one independent version for each variant type (UserAddressV1, UserAddressV2, etc ...).

For the serialization/deserialization we will only serialize/deserialize at top level and so always have the two versions in the bytes or string representations. We will not serialize only one variant or only a version of the variant. (We could think about optimization like having only UserAddress only for some API endpoints but it's for the future).

[damip]

So if I understood correctly, we're implementing this:

enum Address {
    SCAddress(SCAddress),
    UserAddress(UserAddress)
}

// ser/deser to/from bytes/bs58check implemented for Address only
// when serializing, use the following format:
//  [type varint][version varint][bytes]

enum SCAddress {
    SCAddressV1(SCAddressV1)
}

// note that SCAddressV1 is not a hash but a serialized pair (slot, index) 

enum UserAddress {
    UserAddressV1(UserAddressV1)
}

and using the new Address enum instead of the current Address across the codebase

[Ben-PH]

That's my understanding, with the caveat that, for the time being, I'm not taking into consideration the versioning, so:

struct SCAddress {
    slot: Slot,
    idx: u64,
    read_only: bool
}

and where Address is currently used:

fn foo(..., addr: Address) -> Result<...> {
let Address::User(addr) = addr else {
    return Err(/* some sort of TODO message here */);
};
// unchanged code here

The TODO is basically

• if SC/User distinction is a control branch, handle that.
• if it must be a specific variant, change the fn's argument type to UserAddress/SCAddress as appropriate

[Ben-PH]

...and I'll be coordinating with @AurelienFT and @Leo-Besancon with respect to versioning.

My understanding around versioning, is that we are shooting for an approach where we use a macro of some sort, so that we write something that is essentially what I'm currently doing, but with the macro annotations, it expands to what you wrote:

enum SCAddress {
    SCAddressV1(SCAddressV1),
    SCAddressV2(SCAddressV2),
}

[AurelienFT]

Yes on you side @Ben-PH you just implement :

enum Address {
  SCAddress(SCAddress),
  UserAddress(UserAddress)
}

struct SCAddress {
   idx, slot, ..
}

struct UserAddress {
  hash
}

The rest will be handled by the version macro and it follows the pattern you shown @damip . So we are good !

[Ben-PH]

First look at something that sorta works: #3452

something that just occured to me. the smallest varint is I32, and we can probably pack the variant prefix and version into a single VarInt, but that's an optimization. Ill make an issue now so I don't forget.

edit: #3453

[Ben-PH]

Does "read-only" need to be encapsulated in the SC address itself? I would think not, as an SC "location" is independant of if it's read/write or not.

...or could there be two SC's at the same location, one read-only version, one read and write?

[AurelienFT]

Does "read-only" need to be encapsulated in the SC address itself? I would think not, as an SC "location" is independant of if it's read/write or not.

...or could there be two SC's at the same location, one read-only version, one read and write?

There is a comment above the read_only inclusion :

add a flag on whether we are in read-only mode or not to the seed
this prevents read-only contexts from shadowing existing addresses

It avoid the addresses that exists on the ledger to be shadowed in the read_only context by another address creation.

[Ben-PH]

I've made some more progress on the address-enum PR:

• Things are currently split between UserAddress and SCAddress
• There is the AddressTrait trait, which uses generic monomorphisation to handle the match self {Self::UC(.... shenanigans (for the most part)
• A few limitations at the moment.
• the [variant][version][bytes] is holding up. The types aren't to spec yet, but that's not an issue at the moment.

a snippet that show-cases the sort of thing that I like about the design:

impl<T: AddressTrait> Deserializer<T> for AddressDeserializer {
    fn deserialize<'a, E: ParseError<&'a [u8]> + ContextError<&'a [u8]>>(
        &self,
        buffer: &'a [u8],
    ) -> IResult<&'a [u8], T, E> {
        let (rest, _) =
            context("Invalid Address Prefix", char(ADDRESS_PREFIX.into())).parse(buffer)?;
        let (rest, pref) =
            context("Invalid Address Variant Prefix", char(T::PREFIX.into())).parse(rest)?;

        let res = T::from_bytes(rest).map_err(|_| {
            nom::Err::Failure(ContextError::add_context(
                rest,
                "Invalid byte stream",
                ParseError::from_error_kind(rest, nom::error::ErrorKind::Fail),
            ))
        })?;
        Ok((rest, res))
    }
}

What's happening here, is that the trait defines the top-level parsings, i.e. "first it's A, then it's the variant selector, and from there you go into pulling out the bytes". For now, it ignores the version.

Main issue at this point ('tis a big one):

One must manually ensure that all implementors of AddressTrait have unique variant prefixes. One option might be to have a macro such as follows:

addr_variants!(
   User('U'),
   SC('S'),
   CompileFail('U'), // fails because the macro detects duplicate `U`
);

Though this solution feels "clever for the sake of it". I'll try to think of a better option.

I had a quick try with a non-enum unit-struct struct Address<T: AddressTrait>; but that runs into problems where there is no distinction bettween the two addresses (such as when they are used as keys in a collection).

There's a few hypothesis that I have in mind, but I need to grok the patterns of address usage, both in terms of the finer details of the code-base, and the required properties of the block clique architecture.

In any case, it shouldn't be too hard to accomodate the versioning macros based on the quick glance I had late last week.

[AurelienFT]

I think I missed some steps here. Why there is a Trait with functions that have constraint on this trait ? Shouldn't be only an enum that have two variant ? For example the serializer/deserializer should only be implemented for the enum as we said with @damip that we don't wanna serialize/deserialize variants of the enum but the whole enum directly.

[Ben-PH]

#3475 cleans simplifies things. Made the mistake of trying to go complicated too early. I think I at least learned a few things along the way.

Under the hood, there is currently no difference between a user and sc address. With the PreHashed impl, and the heavy use of .copied() that we can use with the user-address because of the known-sized array, things get...nuanced.

There are a couple of questions I need to explore that relate to these two issues. In the meantime, we can bring together the versioning and enum'd address, and then refine the inner details of the address once that's landed.

[AurelienFT]

Yes the goal is to have a first version of this so that we can iterate with the versioning and make the optimizations whenever we have the whole system setup.

[Ben-PH]

Associated PR merged

[AurelienFT]

@Ben-Rey Do you have time before the end of the month to make sure the tooling is ready for this change ? All address will become either "AU..." for user or "AS..." for smart-contract instead of "A.."

[Ben-Rey]

Sure, I will ask some help from the ino team if I need

[AurelienFT]

Ok thank you !