-
Notifications
You must be signed in to change notification settings - Fork 25
Add support for wasm
#852
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
base: master
Are you sure you want to change the base?
Add support for wasm
#852
Conversation
f1a3510
to
20d7872
Compare
# For this reason, we are providing a build of the dependencies instead in | ||
# the "Restore cached deps" step, and we make the check not required. | ||
# When we are able to make this CI check self-sufficient, we should reenable the | ||
# caching and remove the manual restoring of cached deps. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that nix shell isn't caching those dependencies?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is missing a lot https://ci.iog.io/build/8189984#tabs-buildsteps
I think we should try bugging devx for help. Maybe building cardano-wasm with nix could populate IOG cache. But I guess it may be hard to do so with haskell.nix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the nix shell is only caching the build tools. I don't know how to build it for wasm using haskell.nix
, but that would probably solve the issue. There is an effort towards achieving that, but I don't think they got there yet. I can ask devx more directly, and see what they say
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main gripe is throwing exceptions from pure code.
75fc33a
to
39631d0
Compare
Back-up before rebasing/squashing: https://github.com/IntersectMBO/cardano-api/tree/backup/cardano-wasm |
6ef16c2
to
972361b
Compare
|
||
source-repository-package | ||
type: git | ||
location: https://github.com/palas/ouroboros-network.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to get our changes easily merged for IOE libraries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a very nice patch: IntersectMBO/ouroboros-network@ef3e306
But we can try, maybe we can make a better patch
-- * API Information Data Types | ||
|
||
-- | Describes the return type of a method. | ||
data MethodReturnTypeInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ordering of the types in this module make it slightly harder to read. The ApiInfo
definition should be first. I won't block the PR on this.
-- been signed yet. It abstracts over the era of the transaction. | ||
-- It is meant to be an opaque object in JavaScript API. | ||
data UnsignedTxObject | ||
= forall era. UnsignedTxObject (Exp.Era era) [Ledger.WitVKey Ledger.Witness] (Exp.UnsignedTx era) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type is counterintuitive. It's unsigned yet it contains the witnesses in the type. What is the benefit of carrying the witnesses in the UnsignedTxObject
constructor vs just adding the witnesses to the unsigned tx to make a signed tx?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The advantage of having the [Ledger.WitVKey Ledger.Witness]
accumulated instead of looking them up in the transaction is that the transaction doesn't tell you which kind of witness they are once you add them (maybe there is a way to find out and I don't know it). But if we add them to the wrapper then we know they must be shelley witnesses because that is the only way to add them there. So that was my thinking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also the type is opaque, so JS/TS users won't see or be affected by the awkwardness
in UnsignedTxObject era (keyWitnesess ++ [keyWitness]) unsignedTx | ||
|
||
-- | Sign an unsigned transaction using the list of signatories. | ||
signTxImpl :: UnsignedTxObject -> SignedTxObject |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this should be signTxImpl :: [Ledger.WitVKey Ledger.Witness] -> Exp.UnsignedTx era -> SignedTxObject
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is that you can add the witnesses one at a time, so the API is simpler, and we can add functionalities in the future (like addByronKeyWitness
, addScriptWitness
, ...), without breaking the API.
The alternative is to have a function that takes a changing type (like you suggest), made of a list of a complex structured types (the witness object), but I don't think that is a good idea for JavaScript, because types are not checked, so the simpler the parameters the better. And why construct an array of witnesses if you can just add them directly?
On the other hand, it is necessary to apply all witnesses at the same time, because otherwise they get invalidated, so that is why I suggest we just accumulate them and apply them at once when you call signTx
.
This also allows the calculateMinFee
function to count the witnesses for you, instead of you having to provide a number (in most cases).
This is the only issue I'm blocking on: #852 (comment) |
Add `cardano-wasm` work to a subfolder Add development shell for wasm to top-level flake Make HLS ignore `cardano-wasm` subdir Add CI to test compilation to WASM Homogenise Haskell <=> JS type conversion
- Rename `wasm.yml` to `haskell-wasm.yml` - Improve comments and convert them into Haddock - Add expected types to errors - Substitute the `./` with `/` in `.gitignore` - Always store the cabal cache - Use `Coin` instead of `Integer` in API - Remove partial pattern matches - Use type synonyms for JavaScript versions of custom types - Add explanation on how to run the example - Add wasm devShell to Hydra - Add instructions for building without nix - Add `HasCallStack` to functions that throw errors - Update to latest `cardano-api`
- Add functions to create transactions using UnsignedTx - Reorganise modules - Link `cardano-wasm` to checked out version of `cardano-api` - Use static cache as temporary fix - Structure transaction API in Haskell side and clean up - Bridge restructured API to JS and update example - Also delete old API
- Merge cabal.project files remove hie.yaml and update base constraint - Import `project-config` and fix compilation errors and format - Fix for CI errors - Fix type synonym - Split `Cardano.Wasm.JavaScirpt.Bridge` in two and remove obsolete functions - Add `HasCallStack` to `addSimpleTxOutImpl` - Evaluate exceptions eagerly in IO - Reorganise modules - Rename build-complete so that it is not required and disable schedule - Rename CI action for consistency - Add clarifying comment about disabled cache and generalise authors - Move internal modules to `Internal` package - Simplify folder creation in README.md - Rename `APIInfo` to `ApiInfo` - Use `MonadThrow` in partial functions
Changelog
Context
See this ADR: input-output-hk/cardano-node-wiki#75
How to trust this PR
I think the best way is testing it and ensuring the code makes sense. We'll work on adding extensive testing later.
Checklist