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

feat: initial portal docs + minor cleanups #1469

Merged
merged 8 commits into from
Aug 14, 2023

Conversation

LHerskind
Copy link
Contributor

Fixes #1439.

Consider serving locally to review in proper format.

Checklist:

Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge.

  • If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag.
  • I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code.
  • Every change is related to the PR description.
  • I have linked this pull request to relevant issues (if any exist).

@LHerskind LHerskind marked this pull request as ready for review August 9, 2023 15:39
@LHerskind LHerskind enabled auto-merge (squash) August 9, 2023 15:46
Copy link
Member

@Maddiaa0 Maddiaa0 left a comment

Choose a reason for hiding this comment

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

Looks good! Have some grammar fixes

docs/docs/dev_docs/contracts/portals/inbox.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/inbox.md Outdated Show resolved Hide resolved

| Name | Type | Description |
| -------------- | ------- | ----------- |
| Recipient | `L2Actor` | Who is to receive the message. This **MUST** match the rollup version and an Aztec contract that is **attached** to the contract making this call. If the recipient is not attached to the caller, the message cannot be consumed by it. |
Copy link
Member

Choose a reason for hiding this comment

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

Is the L2Actor object defined anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not in this. I was thinking that it might be useful to add the DataStructures separately in its own page.

docs/docs/dev_docs/contracts/portals/inbox.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/inbox.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/outbox.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/outbox.md Outdated Show resolved Hide resolved

## `Entry`

An entry for the messageboxes multi-sets.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
An entry for the messageboxes multi-sets.
An entry for the message box (the entry acts like a multi-set, enabling multiple entries for the same message)

Copy link
Contributor Author

@LHerskind LHerskind Aug 11, 2023

Choose a reason for hiding this comment

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

The entry does not act like a multiset, it is an entry in a multiset.

docs/docs/dev_docs/contracts/portals/data_structures.md Outdated Show resolved Hide resolved

## `L1Actor`

An entity on L1, specifying the address and the chainid for the entity. Used when specifying sender/recipient with an entity that is on L1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
An entity on L1, specifying the address and the chainid for the entity. Used when specifying sender/recipient with an entity that is on L1.
An entity on L1, specifying the address and the chainId for the entity. Used when specifying sender/recipient with an entity that is on L1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure what exactly we want here, often it is just denoted chainid in solidity docs etc, but I prefer chainId.

docs/docs/dev_docs/contracts/portals/data_structures.md Outdated Show resolved Hide resolved

## `L2Actor`

An entity on L2, specifying the address and the chainid for the entity. Used when specifying sender/recipient with an entity that is on L2.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
An entity on L2, specifying the address and the chainid for the entity. Used when specifying sender/recipient with an entity that is on L2.
An entity on L2, specifying the address and the chainId for the entity. Used when specifying sender/recipient with an entity that is on L2.

Copy link
Contributor Author

@LHerskind LHerskind Aug 14, 2023

Choose a reason for hiding this comment

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

Oh, chainid should be replaced by version here actually.

docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
_token,
_to
);
```
Copy link
Contributor

Choose a reason for hiding this comment

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

<3

docs/docs/dev_docs/contracts/portals/main.md Outdated Show resolved Hide resolved
@LHerskind LHerskind merged commit 37316f4 into master Aug 14, 2023
@LHerskind LHerskind deleted the lh/1439-portals-documentation branch August 14, 2023 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Writing docs for portal contracts
3 participants