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

Test Address versionning #3417

Closed
wants to merge 6 commits into from
Closed

Conversation

Leo-Besancon
Copy link
Collaborator

Address::AddressV1 and Address::AddressV2 variants, each made with a different Hash algorithm.

/!\ The code compiles, but there should be some errors in the way we handle both versions!

  • document all added functions
  • try in sandbox /simulation/labnet
  • unit tests on the added/changed features
    • make tests compile
    • make tests pass
  • add logs allowing easy debugging in case the changes caused problems
  • if the API has changed, update the API specification

Address::AddressV1 and Address::AddressV2 variants, each made with a different Hash algorithm.

/!\ The code compiles, but there should be some errors in the way we handle both versions!
@Leo-Besancon
Copy link
Collaborator Author

Some example of refactoring we would have to do:

In massa\massa-ledger-exports\key.rs:

 let limit = ADDRESS_MAX_SIZE_BYTES + 1;
        buffer.extend(&value[..limit]);
        if value[ADDRESS_MAX_SIZE_BYTES] == DATASTORE_IDENT {

The Address size is basically hardcoded in the way we store the ledger.

@Leo-Besancon
Copy link
Collaborator Author

Leo-Besancon commented Jan 19, 2023

Address versioning

  • How do we compute the Address::get_thread() method in a way that does not depend on the version?
  • How can we handle different address versions in our smart-contracts?

First thoughts:

To simplify, we only worry about the transition between two address versions, AddressV0 and AddressV1.

Vocabulary:

  • Legacy address = AddressV0.
  • New address = AddressV1.
  • Legacy contract = Deployed at a legacy address. Can handle AddressV0 only.
  • New contract = Deployed at a new address. Can handle both address types.

Thread handling

Scenario

Alice has 5 coins in her wallet, and maintains two nodes (one using AddressV0 and one using AddressV1).
She signs at the same time two transactions (T0 and T1) from her two addresses (behind the same keypair).

Tx0: Addr(Alice,V0) -> Bob (value of 5 coins)
Tx1: Addr(Alice,V1) -> Charlie (value of 5 coins)

Tx0 and Tx1 are inserted in two different threads Thread 0 and Thread 1 (because the first few bits of each address is different). She is spending the same coins, but the two threads are not aware of that at the beginning.

Here, maybe the double spend would be caught eventually (through cross-thread syncs), but it would lead to a lot of inefficiencies (because it takes too much time to realize some blocks are incompatible and this hinders finality).

Methods to protect against this problem

  1. Keep the previous hash first bits in the newer addresses.

    1. New addresses may not have the following structure:
      AddressV1: [VersionNumber1][HashV1]
      Instead, they would have the following structure, with T the number of bits needed for threads
      AddressV1: [VersionNumber1][HashV0[0..T]][HashV1]
    2. Problems:
      • Ugly code (we have to use two different hashing algos, just to assign correctly each address...)
  2. Use another discriminant for the thread repartition

    • Computed from Public Key? Do we have access to the Public Key every time we need to access this info?

todo!()

  1. A node would only accept Txs made from the new version. This would lead to a lot of breaking changes.

Smart contract handling

Scenario

At T0, Alice publishes a smart contract SC0 that only handles AddressV0.
At T1, Massa develops AddressV1 standard.
At T2, every node on the network has been updated and handles both addresses AddressV0 and AddressV1.

After T2, the smart contract SC0 will probably be affected if:

  • It uses computations made directly from the address bytes. For example, the address byte size could probably change between the two versions.
  • It stores info on a given address (e.g. "address Addr(Bob, V0) owns this NFT"). This means that if the smart contract uses, at some point, Storage.set(X, Addr(Bob, V0)), then Storage.get(X) == Addr(Bob, V0) is true but Storage.get(X) == Addr(Bob, V1) is false.

Bob would need to access the smart contract from his AddressV0, or "update" it to link the two addresses together.

After T2, the smart contract SC0 will NOT be affected if:

  • It does not interact with addresses
  • It only interacts with addresses through Massa SDK features (?)

Argument handling of address types in smart contracts

In legacy contracts

  1. If the legacy contract does not manipulate addresses at all
    No problem.

  2. If the legacy contract uses addresses for Storage.set() and Storage.get(), or other imported functions from massa-as-sdk
    Example: A basic NFT contract. It stores the NFT owner as Storage.
    An user should be able to prove that two addresses are the same?

  3. If the legacy contract handles addresses bytes directly.
    A new version of this contract, handling both address versions, should be deployed.

In new contracts

  1. The new contract should have logic that differenciate two of the same address?

Smart contract addresses access

  1. A legacy contract will always be accessible from its original address.

  2. A new contract will be deployed at a new address format.

    1. A legacy contract won't be able to call a new contract?
    2. A new contract should be able to call a legacy contract.

The address of a smart contract is computed in massa/mass_execution_worker/src/context.rs, in

pub fn create_new_sc_address(&mut self, bytecode: Vec<u8>) -> Result<Address, ExecutionError>

In particular, it is computed has a hash of deterministic data

let mut data: Vec<u8> = self.slot.to_bytes_key().to_vec();
// add the index of the created address within this context to the seed
data.append(&mut self.created_addr_index.to_be_bytes().to_vec());
// 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
if self.read_only {
    data.push(0u8);
} else {
    data.push(1u8);
}
let address = Address::AddressV2(AddressV2(massa_hash::HashV2::compute_from(&data)));

Changing the hash algorithm will produce new contract addresses easily.

Implementation choices

  • Use a Translation map? Such as proposed in these notes: https://notes.ethereum.org/@ipsilon/address-space-extension-exploration#ASE-Address-Space-Extension-with-Translation-Map

    • Users can still use both address versions transparently
    • Additionnal features ( ... )
    • BUT
    • Additionnal storage
    • Not clean
    • Adds support for just 1 additional version (i.e. to support N versions, we need N-1 translation maps).
  • Just consider them as two different addresses!

    • In this case, a user could for example transfer his previous address' NFTs to his new address.
    • Basically, for smart contracts, it would be the same as two different wallets
    • A lot cleaner / simpler
    • BUT
    • No automatic validation that two addresses are the same!

todo!()

References

@damip
Copy link
Member

damip commented Jan 19, 2023

How about forcing simultaneous versioning of addresses + keys ? eg. even if only the hashing algo is updated, the version numbers of newly created keys is also updated. And when the signature algo is updated, also increment the version of the derived addresses.

In that case, the Address structure can be an enum (like https://doc.rust-lang.org/std/net/enum.IpAddr.html ) with the following methods, each managing address types with a match &self:

  • get_size(&self) -> usize
  • get_thread(&self) -> u8

The PublicKey structure can also be an enum, and expose a compute_address(&self) -> Address method that yields the right address version.

Advantage: the "version" is actually the "crypto version" in use (where "crypto" is the (hash, sig) pair of algorithms)

Disvantage: both addresses and keys get an extra enum element even if just one of them changed, and updating address version always means creating a new account.

Still feels like the least error-prone approach

@Leo-Besancon
Copy link
Collaborator Author

Leo-Besancon commented Jan 20, 2023

This seems like a neet idea! @AurelienFT is currently working on a set of macros helping the versioning our various structures (avoiding some code duplication).

I think that versioning the Address and the KeyPair together is a good way to avoid some problems (and to lean into the "the updated address is distinct from the previous one" idea that avoids hacks such as translation maps).

@AurelienFT
Copy link
Contributor

Next steps in #3442 #3471

@AurelienFT AurelienFT closed this Feb 1, 2023
@Leo-Besancon Leo-Besancon deleted the test-versioning-addresses branch April 6, 2023 09:47
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

Successfully merging this pull request may close these issues.

3 participants