ChainId incorporates network/version about chainweb-node HOT 6 CLOSED

Proposal is

export (ChainId,mkChainId) -- private constructor
...
data ChainId = ChainId ChainwebVersion Word32 deriving (Ord...) -- not sure if Ord is a necc. feature of current ChainId

mkChainId :: MonadThrow m => ChainwebVersion Word32 -> m ChainId

from chainweb-node.

In the API, ChainwebVersion and ChainId should go into separate path segments because not all chainweb Resources are chain-resources. So, not all resources have a ChainId, but all have a ChainwebVersion.

So path should still look like .../Testnet00/chain/0/.... This expresses that a chain id can only occur in the scope of a Chainweb version.

from chainweb-node.

I propose that instead of changing the existing ChainId newtype, we introduce a new product type, say, ChainwebChainId, that holds a ChainId and a ChainwebVersion.

We have already a HasChainwebVersion type class. Most chain resources are instances of it (e.g. BlockHeader, TreeDB, BlockHeaderDB). The new type that this issue proposes should be an instance of this class.

Component APIs should use the new ChainwebChainId type. An example would be selecting the source chain in the SPV API. Also, the the public getter for the chain id of a block header (or other data types that contain chain ids) should return this type.

In the chainweb consensus code base we still need the "bare" ChainIdnewtype and I propose to not change that type, because it is used in many places. Instead, I propose to limit its usage to internal, low-level APIs. It would be costly, both on the heap and in serializations to always replicate the chainweb version. It would also slow down code that uses maps that are indexed with chain ids, like, for instance cuts, and vectors of adjacent parent hashes. For data structures that contain those maps it still makes sense to store the chainweb version only once. The public Getter's should still return the "safe" ChainwebChainId type with the HasChainwebVersion instance.

from chainweb-node.

I propose the following change, that allows for a less intrusive implementation.

export (ChainwebChainId,mkChainwebChainId) -- private constructor
...
data ChainwebChainId = ChainwebChainId ChainwebVersion ChainId

mkChainwebChainId :: MonadThrow m => ChainwebVersion ChainId -> m ChainwebChainId
mkChainwebChainId_ :: MonadThrow m => ChainwebVersion Word32 -> m ChainwebChainId

Possibly only one smart constructor is needed.

This also doesn't require to derive all instances that current low-level code requires for ChainId.

In addition the constructor of BlockHeader should be made private (something that we were planning since a while but didn't do to not disturb work on Testnet) and blockChainId should return ChainwebChainId. Other component API types and functions that expose ChainId should be change analogously.

from chainweb-node.

I think that the default way to obtain values of type ChainId in production code, should be either

• via chainIds_ $ _chainGraph version, or
• via validated deserialization (e.g. as part of block headers).

Outside of testing, it should rarely be a need to create a chain id value out of thin air.

In fact, unsafeChainId is currently used only in very places. It's legitimately used in ea and Chainweb.Graph. Otherwise it is used only in test code. Therefor the main use of the new "safe" chain id type would probably be in pact when a user selects a chain for SPV transactions.

from chainweb-node.

It would seem this is mainly for REST API endpoints, which we still need to have a discussion about to handle hardforks. Closing for now.

from chainweb-node.