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

plutus-contract: Reduce our use of row-types, delete BlockchainActions #3342

Merged
merged 19 commits into from
Jun 15, 2021

Conversation

j-mueller
Copy link
Contributor

@j-mueller j-mueller commented Jun 10, 2021

  • Replace the BlockchainActions part of the schema with a pair of types PABReq and PABResp for the requests/responses that Contracts can make
  • Apart from reducing our row-types usage this also makes it much easier to see what actually gets passed to contract instances
  • The schema is now only used for user-defined endpoints
    • Unfortunately the mkSchema TH function cannot handle EmptySchema so I had to put a dummy endpoint in a couple of places in the playground
  • We already had something like this in the PAB, in form of the ContractPABRequest and ContractPABResponse types. I deleted those.

Pre-submit checklist:

  • Branch
    • Commit sequence broadly makes sense
    • Key commits have useful messages
    • Relevant tickets are mentioned in commit messages
    • Formatting, materialized Nix files, PNG optimization, etc. are updated
  • PR
    • Self-reviewed the diff
    • Useful pull request description
    • Reviewer requested

Pre-merge checklist:

  • Someone approved it
  • Commits have useful messages
  • Review clarifications made it into the code
  • History is moderately tidy; or going to squash-merge

@j-mueller j-mueller force-pushed the j-mueller/scp-2018-waiting-actions branch 3 times, most recently from 10afcfb to 9e14c9c Compare June 12, 2021 19:40
@j-mueller j-mueller requested a review from koslambrou June 14, 2021 13:47
@j-mueller j-mueller force-pushed the j-mueller/scp-2018-waiting-actions branch from 54ead2e to 857e869 Compare June 14, 2021 14:04
GenericError e -> pretty e
ThreadIdNotFound i -> "Thread ID not found:" <+> pretty i
InstanceIdNotFound w -> "Instance ID not found:" <+> pretty w
EmulatorJSONDecodingError e v -> "emulator JSON decoding error:" <+> pretty e <+> parens (viaShow v)
Copy link
Contributor

@koslambrou koslambrou Jun 14, 2021

Choose a reason for hiding this comment

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

Capitalize?

import qualified Control.Monad.Freer.Extras.Log as L
import qualified Control.Monad.Freer.Writer as W
import Ledger.AddressMap (UtxoMap)
import Prelude hiding (until)
Copy link
Contributor

Choose a reason for hiding this comment

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

hiding (until) not needed anymore

@koslambrou
Copy link
Contributor

LGTM. Curiously, modules such as Plutus.Contract.Effects.AwaitSlot and Plutus.Contract.Effects.AwaitTxConfirmed were almost completely stripped of it's content, but they still exist. Why's that?

@j-mueller
Copy link
Contributor Author

but they still exist. Why's that?

I meant to delete them when I was done but I forgot. Thanks for pointing it out!

@j-mueller j-mueller marked this pull request as ready for review June 15, 2021 08:59
@j-mueller j-mueller force-pushed the j-mueller/scp-2018-waiting-actions branch from 17f4790 to c650fec Compare June 15, 2021 12:11
@@ -199,7 +189,7 @@ tests =
matchLogs :: [EM.EmulatorTimeEvent ContractInstanceLog] -> Bool
matchLogs lgs =
case (_cilMessage . EM._eteEvent <$> lgs) of
[ Started, ContractLog "waiting for endpoint 1", CurrentRequests [_], ReceiveEndpointCall _, ContractLog "Received value: 27", HandledRequest _, CurrentRequests [], StoppedNoError] -> True
[ Started, ContractLog "waiting for endpoint 1", CurrentRequests [_], ReceiveEndpointCall{}, ContractLog "Received value: 27", HandledRequest _, CurrentRequests [], StoppedNoError] -> True
Copy link
Contributor

Choose a reason for hiding this comment

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

waiting => Waiting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes of course...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm no it passes locally?

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.

2 participants