Deprecate Account model on client.

[jnaviask]

Tracking ticket for understanding requirements to get rid of the Account model on the client, one of the oldest legacy objects in the codebase.

[timolegros]

After reviewing the Account and AddressInfo models I propose the following:

1. Merge Account and AddressInfo into a single AddressAccount model that more or less matches the database model. The Account and AddressInfo models are used interchangeably throughout the client so this makes sense.
2. Remove all inheritance/extensions of Account
   • Refactor the chain accounts such as CosmosAccount which only use this.address (address can be a method argument rather than stateful)
   • The current setup is very confusing. A Chain account such as EthereumAccount stores a reference to all EthereumAccounts thus we have a circular dependency where EthereumAccounts stores all accounts but each account within it stores the original EthereumAccounts themselves. With this change, I propose we deprecate the EthereumAccount but keep the general EthereumAccounts store.
   • Remove IBalanceAccount -> an address can have balances on multiple chains for multiple different tokens so this abstraction is restrictive. Instead, we should fetch balances dynamically by providing the base address, chain, and token to a utility function in the Chain controller for that chain. This change makes it easier to make an interoperable community with arbitrary chains/balances in the future. If we want to cache/store the results of this balance we can create an optional mapping from {[token_id] => balance} on the AddressAccount model.

Naming the new model Address rather than AddressAccount seems appealing but it would conflict heavily with existing address variables. Leaving the name as Account would also be reasonable but I think that Account doesn't properly describe the relationship with the Address DB model.

Pros:

• Cleans up the ambiguity between Account, AddressInfo, and chain accounts like EthereumAccount
• Clarify the front-end data model by removing instances where we extend a data model to create a controller and make it clear that AddressAccount is related to the Address DB model.
• Facilitates multi-chain communities since Addresses won't be associated with a single token.

Cons:

• This will take a while considering the number of objects that extend and utilize Account and AddressInfo

[jnaviask]

I like the plan, although it would be good to know the estimate of how long it would take.

Also, how would we theoretically handle the case of e.g. new AddressInfo(null, address, chain)? Where id is absent. I guess next step would be for you to write out the type you had in mind for AddressAccount so we can evaluate what construction + usage modes it supports.

[timolegros]

I like the plan, although it would be good to know the estimate of how long it would take.

Also, how would we theoretically handle the case of e.g. new AddressInfo(null, address, chain)? Where id is absent. I guess next step would be for you to write out the type you had in mind for AddressAccount so we can evaluate what construction + usage modes it supports.

export type AddressType = {
  // required args
  chain: ChainInfo;
  address: string;

  // optional args
  addressId?: number;
  walletId?: WalletId;
  validationToken?: string;
  sessionPublicAddress?: string;
  validationBlockInfo?: string;
  profile?: Profile;
  keytype?: string;
  userId?: number;

  // flags
  ghostAddress?: boolean;
  ignoreProfile?: boolean;
}

Constructor:

  constructor({
    chain,
    address,
    ghostAddress,
    addressId,
    walletId,
    validationToken,
    sessionPublicAddress,
    validationBlockInfo,
    profile,
    ignoreProfile,
    keytype,
    userId
  }: AddressType) {
    this.keytype = keytype;
    // Check if the account is being initialized from a Community
    // Because there won't be any chain base or chain class
    this.chain = chain;
    this.address = address;
    this._addressId = addressId;
    this._walletId = walletId;
    this._validationToken = validationToken;
    this._sessionPublicAddress = sessionPublicAddress;
    this._validationBlockInfo = validationBlockInfo;
    this.ghostAddress = !!ghostAddress;
    this.userId = userId;
    if (profile) {
      this._profile = profile;
    } else if (!ignoreProfile && chain?.id) {
      this._profile = app.profiles.getProfile(chain.id, address);
    }
  }

addressId is already an optional parameter in Account.ts so new AddressInfo(null, address, chain) translates to

new AddressAccount({address, chain})

[timolegros]

Current bug preventing me from testing voting:

Error: Must have voting balance at proposal start

This is literally all that we get (in a toast) -> no logs.

It originates from packages/commonwealth/client/scripts/controllers/chain/ethereum/compound/chain.ts:

{
...
   voteAmount = await this.compoundApi.Token.getPriorVotes(address, block);
   ...
   return !voteAmount.isZero();
}

Basically, if voteAmount is 0 then an error is thrown in the caller of this function. Why are we checking prior votes before allowing a user to vote on a compound proposal? What am I missing here? The account I use to vote has 1000 COMP delegation + 1000 COMP owned + 10 ETH.

[timolegros]

Current bug preventing me from testing voting:

Error: Must have voting balance at proposal start

This is literally all that we get (in a toast) -> no logs.

It originates from packages/commonwealth/client/scripts/controllers/chain/ethereum/compound/chain.ts:

{
...
   voteAmount = await this.compoundApi.Token.getPriorVotes(address, block);
   ...
   return !voteAmount.isZero();
}

Basically, if voteAmount is 0 then an error is thrown in the caller of this function. Why are we checking prior votes before allowing a user to vote on a compound proposal? What am I missing here? The account I use to vote has 1000 COMP delegation + 1000 COMP owned + 10 ETH.

@jnaviask

[timolegros]

Fixed the blocker.

[jnaviask]

Blocked by: #3040

[CowMuon]

@timolegros What is point value on this story? (Hoping to have you unblocked tomorrow)