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

Sketch of Plutus chain emulator #124

Merged
merged 1 commit into from
Sep 20, 2018

Conversation

michaelpj
Copy link
Contributor

This is an actually-working version of the types @j-mueller and I sketched out yesterday.

I implemented the tiniest part of the emulated wallet (submitting a fake transaction), so there is an actual example you can run! You can load up the file in GHCI and then run runState (runExceptT (process trace)) emptyEmulatorState.

@michaelpj
Copy link
Contributor Author

I mad a number of changes from the draft, but I think they're all simplifications.

-- TODO
class WalletAPI m where
submitTxn :: Txn -> m ()
handleNotifications :: [Notification] -> m ()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this to allow the Plutus client (user of wallet API) to register for notifications? If so then I think it should be possible to register a callback

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is supposed to be the function that gets called on the wallet when some notifications come in for it. At which point it should add any appropriate blocks to its internal state, trigger reactions, etc.

I didn't write in any of the functions for registering handlers etc, but obviously those should be there.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, I thought that we would eventually have two implementations of WalletAPI, namely the mock wallet API and the "real" wallet API. In the real one it wouldn't make much sense to notify the wallet like this (because it would receive its notifications from the core node, which would not be visible to the client).

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe remove it from the class and write

handleNotifications :: MonadEmulator m => [Notification] -> m ()
handleNotifications = ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see. So the version here should just work on the emulated wallet.

Copy link
Contributor

@j-mueller j-mueller left a comment

Choose a reason for hiding this comment

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

I think we should make a clear distinction between MonadEmulator and WalletAPI (in the latter we do not care whether we the code is run against the emulator or against the real wallet). I added comments to explain how that affects the definitions.

-- take place.
data Event n a where
-- these need the coordination code to tell us what to do
WalletAction :: Agent -> n () -> Event n [Txn]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should add another parameter m for the wallet interactions that the client code is allowed to do. Or maybe even

WalletAction :: forall m. WalletAPI m => Agent -> m () -> Event n [Txn]

By constraining it like that, and removing handleNotifications from WalletAPI we ensure that any code we write as a WalletAction can be run against the real wallet later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's exactly what n is, no? Just without the class constraint here.

Copy link
Contributor

@j-mueller j-mueller Sep 20, 2018

Choose a reason for hiding this comment

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

That's exactly what n is, no?

Yes, and in this place n is not right. Currently I can write

trace :: Trace ()
trace = Op.singleton $ WalletAction (Agent 1) (handleNotifications [BlockHeight 100])

Which does not make sense to me. And the two changes I proposed would ensure that this doesn't type check anymore (there are other changes you can make to achieve the same goal, which I would also be happy with :) )

The reason why I think it shouldn't type check is that inside WalletAction we want to write code that is agnostic of the wallet API implementation, so it could be executed against a real wallet also. And in that context handleNotifications does not make sense because the real wallet gets its blockchain notifications from a core node, not from the plutus client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I 100% agree that handleNotifications should not be in WalletApi, but I don't think we need to change Event.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok 👍

-- TODO
class WalletAPI m where
submitTxn :: Txn -> m ()
handleNotifications :: [Notification] -> m ()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe remove it from the class and write

handleNotifications :: MonadEmulator m => [Notification] -> m ()
handleNotifications = ...

Copy link
Contributor

@mchakravarty mchakravarty left a comment

Choose a reason for hiding this comment

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

Looks good, some comments inline.

data Txn = Txn Int
deriving (Show, Eq, Ord)
-- each agent has its own wallet state
data Agent = Agent Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we call this Wallet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure!

-- Basic types (should be replaced by "real" types later)

-- don’t care what these are, some data to allow typeclasses
data Txn = Txn Int
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe call it Tx? This seems to be the standard abbrev and later you use TxPool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

-- these are always responded to in a routine way, check triggers, update wallet state etc.
WalletRecvNotification :: Agent -> [Notification] -> Event n [Txn]
BlockchainActions :: Event n Block
Assertion :: Assertion -> Event n ()
Copy link
Contributor

Choose a reason for hiding this comment

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

Assertions as events seems strange.

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 are events from the emulator's point of view - they are things that we want to occur at a particular point in time in the event stream.

WalletAction :: Agent -> n () -> Event n [Txn]
-- these are always responded to in a routine way, check triggers, update wallet state etc.
WalletRecvNotification :: Agent -> [Notification] -> Event n [Txn]
BlockchainActions :: Event n Block
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't blockchain actions the same as notifications (further up)? Or what other actions do you imagine the blockchain can take that are relevant for us here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Blockchain actions reflect actions that actually change the chain state. Notifications represent that information reaching wallets. Which is a distinct event that can happen later. Also "blockchain actions" here is an event that bundles up a bunch of core-node stuff that could maybe be distinct.

let block = validated
put emState {
emChain = block : emChain emState,
emFailedTxPool = failed ++ emFailedTxPool emState
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you keep failed transactions? For comparison in tests? (The real blockchain just drops them on the floor.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly (there's a brief note on FailedTxn). You might want to assert that a txn failed to validate - you could equate that with saying it's not in the chain or pool, but it seems helpful to have a reason too.

Actually, I think that's premature design, let's drop it for now.

@michaelpj
Copy link
Contributor Author

Comments addressed.

@michaelpj michaelpj merged commit 1036072 into IntersectMBO:master Sep 20, 2018
@michaelpj michaelpj deleted the imp/emulator-sketch branch September 26, 2018 11:33
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