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

added docs folder with docusaurus project #256

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

nhaga
Copy link
Contributor

@nhaga nhaga commented Sep 1, 2023

No description provided.

Copy link
Contributor

@rooooooooob rooooooooob left a comment

Choose a reason for hiding this comment

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

A few minor changes need to be done (see comments - copy-paste errors + a grammar issue).

I don't think there's much utility in the enums / types files so they could be removed.

I know it's probably just moving over old docs to the new format to get a starting structure, but we'll need (in a later PR?) to update the content of the docs that were copy pasted over from the old docs. Most of those were written in the Shelley / Allegra / Mary eras and some things have changed for Alonzo onward.

# Enums

## Certificate

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused as to the point of this file. It seems more like something that should be covered by something automatically generated like what crates.io does with the rust API.

Serialization/deserialization code is automatically generated from
Cardano’s official specification, which guarantees it can easily stay up
to date! We do this using a tool managed by EMURGO & dcSpark called `cddl-codegen`_
which can be re-used for other tasks such as automatically generate a
Copy link
Contributor

Choose a reason for hiding this comment

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

such as automatically generate a

Grammar. I know it was probably wrong wherever it was originally from in the old docs but we might as well fix it here.


## Documentation

This library generates both `Typescript`_ and `Flow`_ type definitions,
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't use flow anymore in the new CML

@@ -0,0 +1,49 @@
{
"name": "cddl-codegen-documentation",
Copy link
Contributor

Choose a reason for hiding this comment

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

cddl-codegen? (I assume a copy-paste error?)

// ],
// },
// ],
copyright: `Copyright © ${new Date().getFullYear()} Milkomeda Foundation. Built with Docusaurus.`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Milkomeda Foundation?

```rust
pub type PolicyID = ScriptHash;
```

Copy link
Contributor

Choose a reason for hiding this comment

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

see comment in enums section

function parseMetadata(metadata) {
// we must check the type first to know how to handle it
switch (metadata.kind()) {
case CardanoWasm.TransactionMetadatumKind.MetadataMap:
Copy link
Contributor

Choose a reason for hiding this comment

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

this code will need to be updated. the variants here are called Map and List now, not MetadataMap.


## A note on fees

Fees is Cardano Shelley are based directly on the size of the final encoded transaction. It is important to note that a transaction created by this library potentially can vary in size compared to one built with other tools. This is because transactions, as well as other Cardano Shelley structures, are encoded using [CBOR](https://cbor.io/) a binary JSON-like encoding. Due to arrays and maps allowing both definite or indefinite length encoding in the encoded transaction created by the library, the size can vary. This is because definite encoding consists of a tag containing the size of the array/map which can be 1 or more bytes long depending on the number of elements the size of the encoded structure, while indefinite length encoding consists of a 1 byte starting tag and after all elements are listed, a 1 byte ending tag. These variances should should only be a couple bytes and cardano-multiplatform-lib uses definite encoding which is the same length or smaller for any reasonable sized transaction.
Copy link
Contributor

Choose a reason for hiding this comment

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

We do match encodings now (when creating via CBOR bytes) but will use definite encodings by default still so maybe this passage is still relevant. It could be worth mentioning this at least though. The "Cardano Shelley" wording as well. I know this was just pulled from the old docs to get the format in but we'll want to look through these moved-over things and make updates like this.

.key_deposit(CardanoWasm.BigNum.from_str('2000000'))
.max_value_size(4000)
.max_tx_size(8000)
.coins_per_utxo_word(CardanoWasm.BigNum.from_str('34482'))
Copy link
Contributor

Choose a reason for hiding this comment

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

the API has been changed. this code must've been from a previous era as we don't have coins_per_utxo_word anymore. There's a coins_per_utxo_byte but I'm guessing there are other areas of this code that needs updating too.

Copy link
Contributor

@rooooooooob rooooooooob left a comment

Choose a reason for hiding this comment

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

I'm just going to approve and merge this PR and do the fixes in a new branch since this is from a fork.

The failing check is happening on develop too and is fixed in #258

@rooooooooob rooooooooob merged commit 858f48c into dcSpark:develop Sep 26, 2023
1 check failed
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