Skip to content

Conversation

@j-mueller
Copy link
Collaborator

No description provided.

@j-mueller j-mueller requested a review from choener May 16, 2025 13:56
Copy link
Collaborator

@choener choener left a comment

Choose a reason for hiding this comment

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

LGTM.

Locally I am still waiting on cache.iog.io, so can't yet try the changes, but they look good.

In case I see something later locally, I'll raise an issue.

import PlutusLedgerApi.V3 (Credential)

data AppError era =
data ProgrammableTokensError =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, it looks like the main error type holds each suberror only once!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep that is the only way to avoid issues with catch, sadly I don't think there is a way to really enforce this

| GlobalParamsNodeNotFound -- ^ The node with the global parameters was not found
| BalancingError (CoinSelection.BalanceTxError era)
| BlockfrostErr BlockfrostError
-- TODO: The following errors are specific to the regulated stablecoin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here one would wrap some RegulatedStablecoinError and these should in turn have AsRegulated... or maybe down in AppError.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I'll split up the type in another PR.


makeClassyPrisms ''ProgrammableTokensError

data AppError era =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Each error type shows up once, good.

deployAll = do
target <- ask
failOnError $ Env.withEnv $ asAdmin @C.ConwayEra $ do
failOnError @_ @(AppError C.ConwayEra) $ Env.withEnv $ asAdmin @C.ConwayEra $ do
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would consiver expanding Convex.MockChain a bit, and provide a MonadError instance.
Then it would not be necessary to call failOnError. A simple newtype for the unit tests in here would suffice.

Though it would not save on lines / characters since instead of MonadFail, we'd carry around a bunch of AsError. However, it would be clear what type of errors are being tested for.

@choener
Copy link
Collaborator

choener commented May 19, 2025

FYI: I get a segmentation fault trying to build WSC via nix.
I'll check on another machine to verify.

@j-mueller
Copy link
Collaborator Author

The build passed so I'm going to merge this now, let me know if the segfault is persistent

@j-mueller j-mueller merged commit ce472f1 into main May 20, 2025
5 checks passed
@j-mueller j-mueller deleted the as-patterns branch May 20, 2025 07:58
@choener
Copy link
Collaborator

choener commented May 20, 2025

The build passed so I'm going to merge this now, let me know if the segfault is persistent

This seems to be weirdly specific. My AMD/Ryzen3 machine produces a segfault, but actually earlier than the wsc code, after checking.
The Intel-based notebook is fine.
I'll probably have the server (Ryzen5) run a build, too. But for now it seems to be a local issue.

@j-mueller
Copy link
Collaborator Author

Hm, on my AMD machine I get an error when trying to nix develop, but it works fine on my other environments

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