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

STS Evolution #1804

Closed
wants to merge 5 commits into from
Closed

STS Evolution #1804

wants to merge 5 commits into from

Conversation

nc6
Copy link
Contributor

@nc6 nc6 commented Aug 21, 2020

Draft PR for discussion.

This presents a straw man for a means to extend base Shelley system with additional modules providing other features. It introduces a basic account based blockchain model, and then a separate module which overrides this to support multi-currency.

This demonstrates the following features:

  • The base system does not need to know anything about the extension - dependencies flow entirely one way. This is not to say that the base code does not have to change, but the only change needed is being more specific about the strict requirements for the base ledger.
  • We allow type overloading on two levels, through having type families which resolve to era-indexed types. By changing the type family instance for a completely new type, we can completely change out types involved. But by simply changing the era parameter, anything which depends only on the structure of the type will continue to work. We show examples of this both in OPS and ADMIN - ADMIN for example relies on the structure of Accounting and Admin but not their specific type:
HasLens (State admin) (Accounting e),
    HasGetter (Signal admin) (Admin e),

processOps relies on the structure of Op, but is fully parametric over its Accounting provided that its tx and admin rules support it, since all processOps does is pass this down to a lower layer.

  • The amount of code which needs to be copied is minimal - mostly just some plumbing code.
  • Size of the extension module is generally small.
  • We're actually closer to the formal ledger in being clear about what our state/signals provide, rather than what they are.
  • Not too many changes needed to start adopting this framework - only the change from data to type for PredicateFailure.

Downsides:

  • A bunch of extra constraints and type annotations to convince things to work.
  • Relies on turning lots of things into type families to be instantiated later. This would be pretty large if we did this wholesale in the Shelley ledger.
  • A lot of the plumbing code is formulaic.

Thoughts:

  • Base monad might be annoying - I think we should aim not to change this.
  • We could probably package a lot of the contraints on rules together.
  • We might be able to auto-derive most of the STS instances, if we work out how to use the era parameter properly on the STS systems.
  • I feel like the lens stuff should already exist somewhere.

Posting here for discussion; thoughts, ideas for how to make this better happily received, especially if they come with code!


deriving stock instance Show (Base.AdminPredicateFailure Era_1)

instance STS ADMIN where
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a case where the extension's ADMIN is just the base ADMIN with the era changed.
If type variable on the rule was brought back, would this instance be free?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it would be free, but we might be able to derive it easily.

Copy link
Contributor

@redxaxder redxaxder Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like...

instance SomeConstraint e => STS (ADMIN e) where
    type Environment (ADMIN e) = ()
    type State (ADMIN e) = BT.Accounting e
    type Signal (ADMIN e) = Base.Admin e

    type PredicateFailure (ADMIN e) = Base.AdminPredicateFailure e

    initialRules = []
    transitionRules = [Base.processAdmin @(ADMIN e) @e]

type instance BT.Op Era_0 = Op Era_0

newtype Accounting e = Accounting
{ unAccounting :: Map (BT.AccountName e) (BT.Account e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do the elements of the map also contain an AccountName? If so, is an invariant of this type that the account name of a key should be the same as the account name of the element at that key? Also if this is correct, why does Account need the account name as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do, it is, and because this ledger was only an example and I didn't stop to make it nice :-D


data Block e = Block
{ blockId :: BT.BlockId e,
blockOps :: [BT.Op e]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are blockOps the block payload (transactions) or is this a different thing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep. I should probably have left it as Tx, but then I had used that for a more specific type and was lazy...

extract tx
)
)
trans @ops (TRC (env, inject txs st, inject xs ops))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the call to trans here means that trans @ops has to modify the entire State tx?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, no, since this is an ops transition rule trans @ops should modify State ops.

@nc6 nc6 requested a review from kevinhammond August 25, 2020 12:50
This demonstrates adding an extension module which introduces
multi-currency.
This allows us to add extra fields.
Copy link
Contributor

@JaredCorduan JaredCorduan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this plan!

@nc6
Copy link
Contributor Author

nc6 commented Sep 1, 2020

Closing this PR now, since it was only up to showcase an idea.

@nc6 nc6 closed this Sep 1, 2020
nc6 added a commit that referenced this pull request Sep 9, 2020
This PR demonstrates a way to parametrise the ledger over value. It
paves the way for overriding this type in subsequent eras, without
making any semantic changes to the Shelley ledger.

Functions in the ledger are now classfied into three groups:
- Those that remain universally quantified over eras.
- Those that have specific requirements on certain era-parametrised
types to work (e.g. that they can be serialised), and
- Those that are specific to Shelley.

There are two examples of the latter: `consumed` and `produced`.

The STS rules are likewise split in two:
- Those that remain universally quantified over eras.
- Those that are specific to Shelley.

The intention is that many will transition to the third (missing)
category, where the actual rules specify their exact requirements on the
era (see #1804 for an example of how to do this). This was not done
because I wanted to get this ready for the meeting and I didn't have
time.

This PR makes some very deliberate design choices:
- The 'Value' type family and any constraints thereon are _not_ lifted
into the 'Era' class. Such would imply universality over eras, which is
too strong a constraint. They would prevent us, for example, from adding to
the `Val` class in future eras, which seems quite plausible.
Furthermore, having more explicit constraints at the use site helps our
understanding of the code; it is much more obvious _when_ we are relying
on certain things, which can be a tool to spot bugs. Note that these
constraints do not propogate wildly, because in general they are
dispatched through the instantiation of the type family to a specific
type in a given era.
- It deliberately removes some instances (`Eq` for example) where it
seems there is no strong reason to have it. The presence of an `Eq`
instance everywhere has bitten us in the past, where for example we have
ended up calling `(==)` on a large part of state in the consensus
integration, which is an expensive operation. Honestly, I suspect I'll
have to add these back to get the tests working, since they rely on
them, but I think there's a decent case for making these instances
orphans in the testing code instead.

This is a *draft* PR for discussion. It does not yet touch the testing
code, and is inconsistent about various things (in particular, I stuck
`era ~ Shelley c` in a lot of places in a hurry).
polinavino pushed a commit that referenced this pull request Sep 24, 2020
This PR demonstrates a way to parametrise the ledger over value. It
paves the way for overriding this type in subsequent eras, without
making any semantic changes to the Shelley ledger.

Functions in the ledger are now classfied into three groups:
- Those that remain universally quantified over eras.
- Those that have specific requirements on certain era-parametrised
types to work (e.g. that they can be serialised), and
- Those that are specific to Shelley.

There are two examples of the latter: `consumed` and `produced`.

The STS rules are likewise split in two:
- Those that remain universally quantified over eras.
- Those that are specific to Shelley.

The intention is that many will transition to the third (missing)
category, where the actual rules specify their exact requirements on the
era (see #1804 for an example of how to do this). This was not done
because I wanted to get this ready for the meeting and I didn't have
time.

This PR makes some very deliberate design choices:
- The 'Value' type family and any constraints thereon are _not_ lifted
into the 'Era' class. Such would imply universality over eras, which is
too strong a constraint. They would prevent us, for example, from adding to
the `Val` class in future eras, which seems quite plausible.
Furthermore, having more explicit constraints at the use site helps our
understanding of the code; it is much more obvious _when_ we are relying
on certain things, which can be a tool to spot bugs. Note that these
constraints do not propogate wildly, because in general they are
dispatched through the instantiation of the type family to a specific
type in a given era.
- It deliberately removes some instances (`Eq` for example) where it
seems there is no strong reason to have it. The presence of an `Eq`
instance everywhere has bitten us in the past, where for example we have
ended up calling `(==)` on a large part of state in the consensus
integration, which is an expensive operation. Honestly, I suspect I'll
have to add these back to get the tests working, since they rely on
them, but I think there's a decent case for making these instances
orphans in the testing code instead.

This is a *draft* PR for discussion. It does not yet touch the testing
code, and is inconsistent about various things (in particular, I stuck
`era ~ Shelley c` in a lot of places in a hurry).
redxaxder pushed a commit that referenced this pull request Sep 25, 2020
This PR demonstrates a way to parametrise the ledger over value. It
paves the way for overriding this type in subsequent eras, without
making any semantic changes to the Shelley ledger.

Functions in the ledger are now classfied into three groups:
- Those that remain universally quantified over eras.
- Those that have specific requirements on certain era-parametrised
types to work (e.g. that they can be serialised), and
- Those that are specific to Shelley.

There are two examples of the latter: `consumed` and `produced`.

The STS rules are likewise split in two:
- Those that remain universally quantified over eras.
- Those that are specific to Shelley.

The intention is that many will transition to the third (missing)
category, where the actual rules specify their exact requirements on the
era (see #1804 for an example of how to do this). This was not done
because I wanted to get this ready for the meeting and I didn't have
time.

This PR makes some very deliberate design choices:
- The 'Value' type family and any constraints thereon are _not_ lifted
into the 'Era' class. Such would imply universality over eras, which is
too strong a constraint. They would prevent us, for example, from adding to
the `Val` class in future eras, which seems quite plausible.
Furthermore, having more explicit constraints at the use site helps our
understanding of the code; it is much more obvious _when_ we are relying
on certain things, which can be a tool to spot bugs. Note that these
constraints do not propogate wildly, because in general they are
dispatched through the instantiation of the type family to a specific
type in a given era.
- It deliberately removes some instances (`Eq` for example) where it
seems there is no strong reason to have it. The presence of an `Eq`
instance everywhere has bitten us in the past, where for example we have
ended up calling `(==)` on a large part of state in the consensus
integration, which is an expensive operation. Honestly, I suspect I'll
have to add these back to get the tests working, since they rely on
them, but I think there's a decent case for making these instances
orphans in the testing code instead.

This is a *draft* PR for discussion. It does not yet touch the testing
code, and is inconsistent about various things (in particular, I stuck
`era ~ Shelley c` in a lot of places in a hurry).
@JaredCorduan JaredCorduan deleted the nc/sts-evolution-2 branch August 16, 2022 13:17
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.

None yet

4 participants