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

Comment PR #502

Draft
wants to merge 386 commits into
base: empty
Choose a base branch
from
Draft

Comment PR #502

wants to merge 386 commits into from

Conversation

nxsaken
Copy link
Contributor

@nxsaken nxsaken commented Jul 3, 2024

PR to hopefully use for comments. Do not merge this.

outoftardis and others added 30 commits May 31, 2022 11:41
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: William Richter <[email protected]>

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
* [documentation]: Review Triggers chapter

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Fix formatting

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Apply suggestions from code review

Co-authored-by: William Richter <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Fix formatting

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Add a link to queries

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

Co-authored-by: William Richter <[email protected]>
Co-authored-by: William Richter <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: William Richter <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: William Richter <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: Aleksandr Petrosyan <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: William Richter <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: Aleksandr Petrosyan <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
[CI]: target `main` branch in GitHub workflows
0x009922 and others added 9 commits February 12, 2024 08:52
* [chore]: update deps

Signed-off-by: Dmitry Balashov <[email protected]>

* [chore]: `type: module`, fixing warning from Vitest

Signed-off-by: Dmitry Balashov <[email protected]>

* [revert]: project is not ready for ESM

Signed-off-by: Dmitry Balashov <[email protected]>

---------

Signed-off-by: Dmitry Balashov <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
…483)

* Updated Rust guide to include information about transferring assets

Signed-off-by: enxtur <[email protected]>

* Apply suggestions from code review

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Update instructions.md

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

* Update instructions.md

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

---------

Signed-off-by: enxtur <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
Signed-off-by: Nurzhan Sakén <[email protected]>
* Replace `docker-compose` (V1) with `docker compose` (V2)
* Fix spelling in bare metal tutorial
* Update src/guide/advanced/running-iroha-on-bare-metal.md

---------

Signed-off-by: Nurzhan Saken <[email protected]>
Signed-off-by: yamkovoy <[email protected]>
Co-authored-by: yamkovoy <[email protected]>
Copy link

vercel bot commented Jul 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
iroha-2-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 4, 2024 10:09am

Comment on lines 7 to 28
```

+-----------------------------------------------+
| |
| +-----------------+ |
| |Domain | |
| +--------------+ | |
| ||Asset | | |
+--+--+ ||Definition(s)| | |
|World| +--------------+ | |
+--+--+ | | |
| +------------+ | |
| ||Account(s)|| | has +-----------+ |
| |------------------------->Signatories| |
| +-----------------+ +-----------+ |
| | |
| | has +--------+ |
| +------->Asset(s)| |
| +--------+ |
+-----------------------------------------------+

```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is changing to a flat model implementation-wise: hyperledger-iroha/iroha#3921, but this diagram is conceptually still valid. Except I believe an account can only have one signatory now: hyperledger-iroha/iroha#4411

Copy link
Contributor

@s8sato s8sato Jul 5, 2024

Choose a reason for hiding this comment

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

Yeah, I think it is a good point to consider keeping conceptual diagrams:
In the future, internal representation of objects may diverge from their appearance at user interface.
For example, the primary key for account storage might be signatory, as opposed to the (UI) account ID signatory@domain, which is btw an intent of this suggestion.
However, it is not the implementation details that should be included in the user guide, but the conceptual diagram at UI

Comment on lines 6 to 8
The assets can be **fungible** (every £1 is exactly the same as every other
£1) or **non-fungible** (a £1 bill signed by the Queen of Hearts is not the
same as a £1 bill signed by the King of Spades).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think asset definitions work like that. We don't have NFTs, only numeric assets (with elastic or fixed supply) and store assets (arbitrary JSON data even within a single definition)

Copy link
Contributor

@s8sato s8sato Jul 5, 2024

Choose a reason for hiding this comment

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

My understanding is that Store/Numeric corresponds to NFT/FT, respectively

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@s8sato I was under the same impression, but then realized that true NFTs should disallow instantiating more than one of an asset definition (monalisa#louvre#alice@wland and monalisa#louvre#bob@wland cannot co-exist since there is only one Mona Lisa in Louvre).

BTW, I think that Store assets, as designed right now, don't make much sense because two instances of the same definition can have totally different keys.

Copy link
Contributor

@s8sato s8sato Jul 5, 2024

Choose a reason for hiding this comment

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

Indeed, we might move away from the idea of representing Store by a combination of a shared definition and its own value.
Maybe ideally the identifier should be smt like a hash of the value.
We may move this discussion somewhere else, maybe hyperledger-iroha/iroha#4087 or hyperledger-iroha/iroha#4782 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Apart from that, as for this doc, what do you think about not mentioning to FT/NFT but just mentioning it is planned to be implemented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can omit NFT/FT and focus on the distinction between Numeric (with elastic or fixed global supply) and Store assets for now

same as a £1 bill signed by the King of Spades).

The assets can also be **mintable** (you can make more of them) and
**non-mintable** (you can only specify their initial quantity in the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Assets with a fixed supply can be minted once, but it doesn't have to be in the genesis. We should also rethink "non-mintable" as it really means "mintable but only once"

Comment on lines 16 to 22
Additionally, the assets have different underlying value types.
Specifically, we have `AssetValueType.Quantity`, which is effectively an
unsigned 32-bit integer, a `BigQuantity`, which is an unsigned 128-bit
integer, and `Fixed`, which is a positive (though signed) 64-bit
fixed-precision number with nine significant digits after the decimal
point. All three types can be registered as either **mintable** or
**non-mintable**.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We only have Numeric now, which is backed by the rust_decimal crate.

Comment on lines 24 to 26
There is also the `Store` asset type, which is used for storing key-values
in object's metadata. We talk in detail about `Store` asset in the chapter
related to [metadata](metadata.md).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit confusing. Store assets contain Metadata (a Metadata is just a mapping between keys and JSON strings, IMO we should rename this to smth like JsonMap), and most objects also contain separate Metadata (same key-JSON map type) but unrelated to assets.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, "metadata" sounds secondary, but it should be primary in Store assets

@@ -0,0 +1,17 @@
# Expressions, Conditionals, Logic
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Outdated section. This was replaced by Custom Instructions, which should be documented with the Executor probably.

| [Grant/Revoke](#grant-revoke) | Give or remove certain permissions from accounts. |
| [Transfer](#transfer) | Transfer assets between accounts. |
| [ExecuteTrigger](#executetrigger) | Execute triggers. |
| [If, Pair, Sequence](#composite-instructions) | Use to create composite instructions. |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed


- domains
- accounts
- assets
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only store assets

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, but it's not exactly what attached to the main object

Comment on lines 13 to 24
The metadata can be of very different types, such as:

- structures with named or unnamed fields
- enums
- integers
- numbers with fixed decimal precision
- strings
- Boolean values
- arrays
- associative arrays
- vectors
- request results
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Metadata maps keys to JSON strings: hyperledger-iroha/iroha#4353

- Iroha [configuration parameters](./../configure/client-configuration.md)
- the list of
[trusted peers](/guide/configure/peer-configuration#trusted-peers)
- registered domains
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 have been unnested from domains, see hyperledger-iroha/iroha#3921:

  • accounts
  • asset definitions
  • assets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be renamed according to hyperledger-iroha/iroha#4810, see the change in the README for smart contracts

* Getting started update and overhaul

Signed-off-by: a_zorina <[email protected]>
Signed-off-by: a-zorina <[email protected]>

* fix broken links

Signed-off-by: a_zorina <[email protected]>
Signed-off-by: a-zorina <[email protected]>

* fix section links

Signed-off-by: a_zorina <[email protected]>
Signed-off-by: a-zorina <[email protected]>

* Language and structure improvements

Signed-off-by: a-zorina <[email protected]>

* External links and crate names fixes

Signed-off-by: a-zorina <[email protected]>

* Switch to using cargo install for iroha binaries in getting started

Signed-off-by: a-zorina <[email protected]>

* Minor post-review fixes

Signed-off-by: a-zorina <[email protected]>

* Fixes and suggestions after review

Signed-off-by: yamkovoy <[email protected]>

* Changes after discussion

Signed-off-by: yamkovoy <[email protected]>

* Minor tweaks in instructions

Signed-off-by: yamkovoy <[email protected]>

* Additional post-review updates

Signed-off-by: a-zorina <[email protected]>

* link fixes

Signed-off-by: a-zorina <[email protected]>

* final post-review changes

Signed-off-by: a-zorina <[email protected]>

---------

Signed-off-by: a_zorina <[email protected]>
Signed-off-by: a-zorina <[email protected]>
Signed-off-by: yamkovoy <[email protected]>
Co-authored-by: yamkovoy <[email protected]>
* Update new structure with recent changes

Signed-off-by: a-zorina <[email protected]>

* tweaks to structure after discussions

Signed-off-by: a-zorina <[email protected]>

* tweaks to Iroha Explained section

Signed-off-by: a-zorina <[email protected]>

* format fix

Signed-off-by: a-zorina <[email protected]>

---------

Signed-off-by: a-zorina <[email protected]>
* link fixes due to migration

Signed-off-by: a-zorina <[email protected]>

* Update src/guide/tutorials/rust.md

Signed-off-by: Ekaterina Mekhnetsova <[email protected]>

---------

Signed-off-by: a-zorina <[email protected]>
Signed-off-by: Ekaterina Mekhnetsova <[email protected]>
Co-authored-by: Ekaterina Mekhnetsova <[email protected]>
misc: update dependencies

Signed-off-by: qua <[email protected]>
Signed-off-by: tarmovannas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.