New Peer System.

[dav1app]

This is an attempt to ArkEcosystemArchive/tier-0-program#9

For models, I'm using the GraphQL SDL to show the model since it is less verbose than the BaseModel included on the app and it is almost self-explanatory (but you can use this article as a cheat sheet).

New Peer and CurrentPeer classes

Current Model

type Peer {
   height: Integer! @default(value: null) # String or Null
   host: String
   ip: String!
   port: Integer! # Also accepts null
   isCustom: Boolean   
   isHttps: Boolean
   lastUpdated: DateTime! @UpdateAt # Also accepts null
   latency: Integer!
   status: String # Also accepts Integer
   p2pport: Integer @default(value: null) # Integer or Null
}

Proposal

type Peer {
   url: URL!
   version: Integer!
   tag: [String!]
   status: Status
}

type Status {
   lastUpdate: DateTime! @UpdateAt
   height: Integer!
   latency: Integer!
   statusCode: StatusCode!
}

type StatusCode {
  message: String!
  code: Int
}

• The URL class comes from the W3 living standard and implemented in JS using the URL() class. This prevents a lot of mistakes from schemas, ports, IPs (including IPv6). Remember that the wallet should not validate hostnames and hosts directly.
• Separate what is static and dynamic inside the peer.
• Includes a tag instead of isCustom. We can insert a tag default, for example, for peers that come from config/peers/mainnet.json. We can set as many tags as we want to filter and group peers.
• Status code makes easy to create an independent store for peer status. This can be used together with HTTP codes for external peers.

New PeerList class

Current

Today it is simply a vector.

Proposal

PeerLists is a class that holds Peers. When retrieving peers from seed servers, or accessing the configs, or simply accessing the state/store for Peers you are dealing with PeerLists.

type PeerList {
   lastUpdate: DateTime! @UpdateAt
   peers: [Peer!]!
}

This was one of the major problems with this old model since it is hard to distinguish methods from Peers and PeerLists inside the store.

• Separate filter, order and random getters from PeerList. For example:

const peerlist = new PeerList(...)
peerlist.best()
peerlist.broadcast()
peerlist.random(10)
peerlist.refresh()
peerlist.put(new Peer({...}))

• Uniform PeerList origins (config, network or store) and create proper handlers for those peers.

Other features

• Utilities and services specifically for sanitization, internal and external validation for Peers.
• An InputPeer to handle the input properly.
• A ModalPeerTag to make it easy to edit tags inside peers.
• Start creating components and modules with multiple files to make it more maintainable.
• Create default routines when starting the application, syncing, etc.

[alexbarnsley]

Looks good! It may be worth including the improvements to custom peers (e.g. persistence) which is mentioned in ArkEcosystemArchive/tier-0-program#9 just to keep it in one place

[dav1app]

I'm going to use this issue to document my progress. Since I do not understand the system deeply enough to assume the behavior of every component, this clears the path for further abstractions.

A lot of work has been done to make this system more readable and maintainable, even if it breaks some of the patterns inside the code. For example, most of the validations are made inside the component methods directly. I'm implementing the validations separated from the components.

New Classes

Peer
CurrentPeer extends Peer
SeedPeer extends Peer
PeerList

New store model

type PeerStore {
   discovery: PeerDiscoveryInstance,
   current: CurrentPeer,
   lists: [PeerList] 
}

{
   ...
   peer: {
      discovery: PeerDiscovery(),
      current: CurrentPeer(),
      lists: [
         PeerList({
            id: 'ark.devnet', // Added the `chainnet` ID reference to the PeerList
            lastUpdate: Datetime()
            peers: [Peer()]
         })
      ]
   }
   ...
} 

Format peer status (wip)

OK
NO_CONNECTION (all HTTP coddes here?)
WRONG_NETWORK (only if it is forced to ignore everything)
STATUS_CHECK_FAILED (request was done, but something happens. HTTP codes?). 

[dav1app]

Peer shorthands

Current	Shorthand	Method
setCurrentPeer	Peer.connect()	Peer.setCurrent()
connectToBest	PeerList.best().connect()	PeerList.getTop(10).getRandom(1).setCurrent()
ensureStillValid	Peer.update()	Peer.getStatus().setStatus()
updateCurrentPeerStatus	Peer.update()	Peer.getStatus().setStatus()
clientServiceFromPeer	Need to refactor ClientService	Peer.getClientService()
validatePeer	Peer.validate()	Peer.getNetwork().getStatus()

PeerList shorthands

All those methods can be called natively by using array methods on PeerList.peers

Current	Shorthand	Method
get.all	PeerList.all()	PeerList.getAllPeers()
get.ip	PeerList.ip()	PeerList.getPeerByIp(ip)
get.best	PeerList.best()	PeerList.getTop(10).getRandom(1)
get.randomPeers	PeerList.random()	PeerList.deleteCurrent().getRandom(n)
get.randomSeedPeers	PeerList.random({seed: true})	PeerList.getSeedPeers().deleteCurrent().getRandom(n)
get.broadcastPeers	PeerList.broadcast()	PeerList.getBroadcastPeers()
refresh	PeerList.refresh()	PeerList.getPeerListFromNetwork()

[dated]

Chiming in with a little feature request that could be of interest to you: add the option to filter peers by their use of estimates or not.

Unfortunately that information is available only in the meta object of an API request, e.g.

{
  "meta": {
    "totalCountIsEstimate": false, <-- this
    "count": 100,
    "pageCount": 39244,
    "totalCount": 3924387,
    "next": "/transactions?transform=true&page=2&limit=100",
    "previous": null,
    "self": "/transactions?transform=true&page=1&limit=100",
    "first": "/transactions?transform=true&page=1&limit=100",
    "last": "/transactions?transform=true&page=39244&limit=100"
  },
  "data": []
}

These nodes do not return all transactions and are affected by all kind of weird behaviour (pagination not working reliably, transaction API endpoints returning zero results in some cases, etc ...).

Related issue #1335.

[dav1app]

Report

What has been done so far:

• Removed duplications: Notice that some of them still remain due to the necessity of checking requirements (especially the current network id). I'm still thinking about the best way to manage external states without being redundant without forcing a proxy for the module.
• Renamed most of the functions: Names like getters.get are removed, and replaced to getters.filter() (this case let you filter by any parameter instead of IP, helping with the issue mentioned before). I'm organizing it in a way that class names are abstracted through the store modules (eg: peer/current/update() is meaningful for the developers.
• Commented and documented the code. Most of it was already done but doesn't mention possible failures and parameters. Also including some comments over some important behaviors to clarify things for new developers.
• Unit tests: It tests each getter, mutation, and action individually and test every case for the endpoint, with every parameter. Some of the fixtures are replaced by a generator to avoid bias over them. I'm working on errors now.
• Pluggable logger: The ideal scenario is one that both error handling and loggers are handled by a single module over the application. What I'm doing now is removing instances of errors and returning a single logger.
• Optional chaining, since typescript-eslint needs to be upgraded to use it, through a callback function. Now it can fail if any sub-property doesn't exist without TypeErrors. I understand that this change is controversial and this is why I made it easy to be reverted.
• Consistent parameter support. Functions used like best(true, null) are changed to best({ ignoreCurrent: true, networkId: 'name'}). This makes new features easier to be implemented and functions more clear to the developer. Some of the behaviors are normalized (Eg: calling a function without providing networkId aways return the result for currentNetworkId()) .
• Consistent returned values: This is taking some time because some of these values are used by other functions without any documentation. This could be hell when setting types.

Need to do:

• Striping into submodules: Only after finish refactoring the actions. My attempt to do that before the refactor failed due to the coupling of the code. peer/seed and peer/current are good examples of submodules that can be abstracted over the main module.
• Changing the model: I partially did it, but it needs to be done with care to avoid compatibility issues between received peers. A simple change like getting the base URL can break old peers if no failsafe was created. Since vuex isn't prepared to handle instances of a class, this can make the work a bit harder to do but it also makes it a lot more consistent.
• Positive feedback to the user: I'm still mapping when it should be used. This can be improved in the whole application as well. Things like changing the peer can send some positive feedback to the user.

Postponed

Some of the features that I've mentioned earlier required a previous refactor to be implemented (like tags and URL()), but some are coming almost naturally with the current refactor, (like better InputPeer, sanitization and routines).

[faustbrian]

Peer related logic for wallets will be moved to https://github.com/ArkEcosystem/platform-sdk.