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

docs: update RFC 003: Crosslang Message Packet and Error Handling Specs #21548

Closed
wants to merge 12 commits into from

Conversation

aaronc
Copy link
Member

@aaronc aaronc commented Sep 4, 2024

Description

This updates RFC 003 with the first draft of the message packet layout specification. I expect this might change a bit as we iterate, but hopefully it's a good starting point.


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

Summary by CodeRabbit

Release Notes

  • New Features
    • Added a state transaction management system using begin_tx, commit_tx, rollback_tx, and discard_cleanup methods to ensure state consistency in the presence of errors.
  • Documentation
    • Updated the specification for message packets to improve clarity and structure for cross-language and cross-VM communication.
    • Introduced a detailed header layout for message packets, including essential fields and their purposes.
    • Added a new section on error codes and handling, clarifying return values and expected behavior during error scenarios.
    • Elaborated on gas management, specifying the impact of gas limits on execution.

Copy link
Contributor

coderabbitai bot commented Sep 4, 2024

Walkthrough

Walkthrough

The document has been updated to enhance the specification of message packets for cross-language and cross-VM communication. The changes include revising the definition of a message packet to a minimum size of 512 bytes, introducing a detailed header layout with various fields, and adding new sections on error codes and handling, as well as gas management details.

Changes

Files Change Summary
docs/rfc/rfc-003-crosslang.md Revised message packet definition to a minimum of 512 bytes, added detailed header layout, introduced error codes and handling, and expanded on gas management.

Sequence Diagram(s)

sequenceDiagram
    participant A as Sender
    participant B as Receiver
    participant C as MessagePacket

    A->>C: Create Message Packet
    C-->>A: Return Packet
    A->>B: Send Packet
    B->>C: Receive Packet
    C-->>B: Acknowledge Receipt
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@aaronc aaronc marked this pull request as ready for review September 4, 2024 17:53
@aaronc aaronc requested a review from a team as a code owner September 4, 2024 17:53
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (2)
docs/rfc/rfc-003-crosslang.md (2)

243-256: Improve the formatting consistency of the unordered list.

Please consider the following suggestions to improve the formatting consistency of the document:

  • Use asterisks instead of dashes for the unordered list items.
  • Surround the unordered list with blank lines.

Apply this diff to address the formatting issues:

-**Message packets** always start with the 512-byte header with the following layout.
-- **message name**: a 128-byte array with the first byte indicating the length of the message name
-- **self-address**: a 64-byte array with the first byte indicating the length of the address
-- **caller-address**: a 64-byte array with the first byte indicating the length of the address
-- **context token**: a 32-byte array
-- **state token**: a 32-byte array
-- **message name hash**: the first 8-bytes of the SHA256 hash of the message name, which can be used for simplified routing
-- **gas limit**: an unsigned 64-bit integer
-- **input data pointer 1**: 16 bytes, see below for the **data pointer** spec
-- **input data pointer 2**: 16 bytes
-- **output data pointer 1**: 16 bytes
-- **output data pointer 2**: 16 bytes
-- remaining bytes: reserved for future use, should be zeroed when a packet is initialized
+**Message packets** always start with the 512-byte header with the following layout.
+
+* **message name**: a 128-byte array with the first byte indicating the length of the message name
+* **self-address**: a 64-byte array with the first byte indicating the length of the address
+* **caller-address**: a 64-byte array with the first byte indicating the length of the address
+* **context token**: a 32-byte array
+* **state token**: a 32-byte array
+* **message name hash**: the first 8-bytes of the SHA256 hash of the message name, which can be used for simplified routing
+* **gas limit**: an unsigned 64-bit integer
+* **input data pointer 1**: 16 bytes, see below for the **data pointer** spec
+* **input data pointer 2**: 16 bytes
+* **output data pointer 1**: 16 bytes
+* **output data pointer 2**: 16 bytes
+* remaining bytes: reserved for future use, should be zeroed when a packet is initialized
+
Tools
Markdownlint

244-244: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


245-245: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


246-246: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


247-247: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


248-248: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


249-249: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


250-250: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


251-251: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


252-252: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


253-253: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


254-254: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


255-255: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


244-244: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


259-262: Improve the formatting consistency of the unordered list.

Please consider the following suggestions to improve the formatting consistency of the document:

  • Use asterisks instead of dashes for the unordered list items.
  • Surround the unordered list with blank lines.

Apply this diff to address the formatting issues:

-A **data pointer** is specified as follows:
-- **native pointer**: 8-bytes, a pointer to a separate buffer in the native environment or zero
-- **length**: unsigned 32-bit integer
-- **capacity or offset**: unsigned 32-bit integer
+A **data pointer** is specified as follows:
+
+* **native pointer**: 8-bytes, a pointer to a separate buffer in the native environment or zero
+* **length**: unsigned 32-bit integer
+* **capacity or offset**: unsigned 32-bit integer
+
Tools
Markdownlint

260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 5805792 and 1b16334.

Files selected for processing (1)
  • docs/rfc/rfc-003-crosslang.md (2 hunks)
Additional context used
Path-based instructions (1)
docs/rfc/rfc-003-crosslang.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

Markdownlint
docs/rfc/rfc-003-crosslang.md

244-244: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


245-245: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


246-246: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


247-247: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


248-248: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


249-249: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


250-250: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


251-251: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


252-252: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


253-253: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


254-254: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


255-255: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


244-244: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


260-260: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


273-273: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Additional comments not posted (3)
docs/rfc/rfc-003-crosslang.md (3)

6-6: LGTM!

The changelog entry is correctly formatted and provides relevant information about the changes made to the document.


236-240: LGTM!

The code segment provides a clear and concise specification for the message packet layout, including the minimum size, header layout, and the possibility of larger packets depending on message handler needs. It also mentions that message handler specifications should describe the precise utilization of the message packet.


272-279: LGTM!

The code segment provides clear and concise steps for passing a packet from one environment to another, including allocating the packet in the new environment, copying the header, handling data pointers, and updating output data pointers after execution.

Tools
Markdownlint

273-273: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

@aaronc aaronc changed the title docs: update RFC 003: Crosslang Message Packet Spec docs: update RFC 003: Crosslang Message Packet and Error Handling Specs Sep 4, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (4)
docs/rfc/rfc-003-crosslang.md (4)

160-160: Use consistent unordered list item markers.

For consistency with the rest of the document and to adhere to the Markdownlint rule MD004, consider using an asterisk (*) instead of a dash (-) for the unordered list item marker.

-  - `discard_cleanup(state token)`: called by the hypervisor when a call fails due to a fatal unwinding error (such as out of gas), with any state tokens that were used in the call stack that is unwound. This allows for a graceful cleanup of state resources in the event of fatal errors.
+  * `discard_cleanup(state token)`: called by the hypervisor when a call fails due to a fatal unwinding error (such as out of gas), with any state tokens that were used in the call stack that is unwound. This allows for a graceful cleanup of state resources in the event of fatal errors.
Tools
Markdownlint

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


165-189: Clear explanation of the state token and its volatility.

The segment provides a detailed and easy-to-understand explanation of the state token, its volatility, and how it is used by the hypervisor and state modules.

Consider adding a comma after "If the high bit is set" at line 168 to improve readability:

-If the high bit is set then the state token is **volatile**, and if it is unset, it is **readonly**.
+If the high bit is set, then the state token is **volatile**, and if it is unset, it is **readonly**.

As an optional suggestion for conciseness, you may consider rephrasing the sentence at line 172:

-It is possible that state modules expose methods for creating new **state tokens**
+State modules may expose methods for creating new **state tokens**
Tools
LanguageTool

[typographical] ~168-~168: Consider adding a comma.
Context: ... the state token. If the high bit is set then the state token is volatile, and if...

(IF_THEN_COMMA)


[style] ~172-~172: For conciseness, try rephrasing this sentence.
Context: ...rdinate around the usage of this token. It is possible that state modules expose methods for creating new **state tokens...

(MAY_MIGHT_BE)


[style] ~176-~176: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... describes a message handler's behavior with respect to state and side effects. It is an enum v...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[uncategorized] ~179-~179: Loose punctuation mark.
Context: ... both read and write state. * readonly: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~180-~180: Loose punctuation mark.
Context: ...re expected to only read state. * pure: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)

Markdownlint

178-178: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


244-289: Clear and detailed message packet specification.

The segment provides a well-structured and comprehensive specification of the message packet layout, including the header fields, data pointers, and the steps for passing a packet between environments.

To maintain consistency with the Markdownlint rules (MD004 and MD032) and improve the readability of the document, consider the following changes:

  1. Use asterisks (*) instead of dashes (-) for the unordered list items.
  2. Surround the lists with blank lines.

For example:

-  - **message name**: a 128-byte array with the first byte indicating the length of the message name
-  - **self-address**: a 64-byte array with the first byte indicating the length of the address
+
+  * **message name**: a 128-byte array with the first byte indicating the length of the message name
+  * **self-address**: a 64-byte array with the first byte indicating the length of the address
Tools
Markdownlint

252-252: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


253-253: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


254-254: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


255-255: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


256-256: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


257-257: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


258-258: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


252-252: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


269-269: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


282-282: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


290-343: Comprehensive error codes and handling specification.

The segment provides a well-structured and detailed specification of the error codes and handling for the system, covering system-level and handler-level error codes, error data, and unwinding errors.

To improve readability, consider adding a comma before "and" at line 292:

-All invoke and handler methods return a 32-bit unsigned integer error code.
+All invoke and handler methods return a 32-bit unsigned integer error code, and
 if the error code is zero, then the operation was successful.

To adhere to the Markdownlint rule (MD022) and maintain consistency, consider surrounding the heading at line 295 with blank lines:

+
 #### System-Level Error Codes
+
 The following non-zero error codes are defined at the system level:
Tools
LanguageTool

[uncategorized] ~292-~292: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... ### Error Codes & Handling All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)

Markdownlint

295-295: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


297-297: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1b16334 and 8a67a66.

Files selected for processing (1)
  • docs/rfc/rfc-003-crosslang.md (3 hunks)
Additional context used
Path-based instructions (1)
docs/rfc/rfc-003-crosslang.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

LanguageTool
docs/rfc/rfc-003-crosslang.md

[style] ~162-~162: The verb “get” can be informal. Consider replacing it with a form of “to be”.
Context: ...unt handler's metadata can define which get passed to the state handler when an accoun...

(GET_USED_ARE_USED)


[typographical] ~168-~168: Consider adding a comma.
Context: ... the state token. If the high bit is set then the state token is volatile, and if...

(IF_THEN_COMMA)


[style] ~172-~172: For conciseness, try rephrasing this sentence.
Context: ...rdinate around the usage of this token. It is possible that state modules expose methods for creating new **state tokens...

(MAY_MIGHT_BE)


[style] ~176-~176: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... describes a message handler's behavior with respect to state and side effects. It is an enum v...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[uncategorized] ~179-~179: Loose punctuation mark.
Context: ... both read and write state. * readonly: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~180-~180: Loose punctuation mark.
Context: ...re expected to only read state. * pure: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~292-~292: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... ### Error Codes & Handling All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)

Markdownlint
docs/rfc/rfc-003-crosslang.md

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


178-178: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


252-252: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


253-253: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


254-254: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


255-255: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


256-256: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


257-257: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


258-258: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


295-295: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


252-252: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


269-269: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


282-282: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


297-297: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Additional comments not posted (3)
docs/rfc/rfc-003-crosslang.md (3)

6-6: Changelog entry looks good.

The added changelog entry provides a clear and concise summary of the changes made to the document on the specified date.


344-358: Clear gas management specification.

The segment provides a concise and easy-to-understand explanation of how gas is measured and managed in the system, covering the behavior when gas limits are exceeded, how gas limits are inherited or set for nested calls, and how gas consumption is tracked and reported.


Line range hint 360-390: Balanced discussion of the consequences.

The segment provides a well-rounded discussion of the consequences of the proposed design, covering the impact on backwards compatibility, the positive aspects of enabling cross-language and cross-VM modules, the potential negative impact on performance, and the neutral aspect of the design not precluding the continued use of the existing Golang native keeper passing. The discussion is balanced and informative, helping readers understand the implications of the proposed design.

Tools
LanguageTool

[uncategorized] ~292-~292: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... ### Error Codes & Handling All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)

Markdownlint

252-252: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


253-253: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


254-254: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


255-255: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


256-256: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


257-257: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


258-258: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


295-295: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


252-252: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


269-269: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


282-282: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


297-297: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range, codebase verification and nitpick comments (4)
docs/rfc/rfc-003-crosslang.md (4)

6-6: Update Changelog Date Format for Consistency

The changelog entry for 2024-09-04 uses a different date format compared to the previous entry. It's recommended to maintain consistency in date formats throughout the document.

Consider changing the date format to match the previous entries for consistency:

- 2024-09-04: Added message packet, error handling, and gas specifications

160-164: Correct List Syntax for Consistency

The list items in the state handler methods section use dashes instead of asterisks, which is inconsistent with other lists in the document.

To maintain consistency in list item markers throughout the document, consider changing dashes to asterisks:

* `begin_tx(state_token)`: creates a nested transaction within the specified state token
* `commit_tx(state token)`: commits any state changes in the current nested transaction of the specified state token
* `rollback_tx(state token)`: discards any state changes in the current nested transaction of the specified state token
* `discard_cleanup(state token)`: cleans up and discards the current state token
Tools
Markdownlint

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


161-161: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


162-162: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


163-163: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


185-196: Add Blank Lines Around Lists for Markdown Lint Compliance

The list describing the volatility levels lacks blank lines before and after, which is required for proper markdown formatting.

To comply with markdown linting rules and improve readability, add blank lines around the list:

**Volatility** describes a message handler's behavior with respect to state and side effects. It is an enum value that can have one of the following values:

* `volatile`: the handler can have side effects and send `volatile`, `readonly` or `pure` messages to other accounts. Such handlers are expected to both read and write state.
* `readonly`: the handler cannot cause effects side effects and can only send `readonly` or `pure` messages to other accounts. Such handlers are expected to only read state.
* `pure`: the handler cannot cause any side effects and can only call other pure handlers. Such handlers are expected to neither read nor write state.

The hypervisor will enforce **volatility** rules when routing messages to account handlers.
Tools
LanguageTool

[uncategorized] ~186-~186: Loose punctuation mark.
Context: ... both read and write state. * readonly: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...re expected to only read state. * pure: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)

Markdownlint

185-185: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


251-272: Clarify Message Packet Header Layout

The section detailing the message packet header layout is crucial for understanding how data is structured. However, the use of dashes instead of asterisks for list items may cause formatting issues in some markdown parsers.

To ensure the list is correctly formatted across all markdown viewers, consider changing dashes to asterisks:

#### Message Packet Header

**Message packets** always start with the 512-byte header with the following layout.
* **message name**: a 128-byte array with the first byte indicating the length of the message name
* **self-address**: a 64-byte array with the first byte indicating the length of the address
* **caller-address**: a 64-byte array with the first byte indicating the length of the address
* **context token**: a 32-byte array
* **state token**: a 32-byte array
* **message name hash**: the first 8-bytes of the SHA256 hash of the message name, which can be used for simplified routing
* **gas limit**: an unsigned 64-bit integer
* **gas consumed**: an unsigned 64-bit integer
* **input data pointer 1**: 16 bytes, see below for the **data pointer** spec
* **input data pointer 2**: 16 bytes
* **output data pointer 1**: 16 bytes
* **output data pointer 2**: 16 bytes
* remaining bytes: reserved for future use, should be zeroed when a packet is initialized
Tools
Markdownlint

259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


265-265: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


266-266: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


267-267: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


268-268: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


259-259: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Review details

Configuration used: .coderabbit.yml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 8a67a66 and 6ee3467.

Files selected for processing (1)
  • docs/rfc/rfc-003-crosslang.md (3 hunks)
Additional context used
Path-based instructions (1)
docs/rfc/rfc-003-crosslang.md (1)

Pattern **/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"

LanguageTool
docs/rfc/rfc-003-crosslang.md

[uncategorized] ~167-~167: Possible missing comma found.
Context: ...hat each account handler's metadata can define which get passed to the **state handler...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~167-~167: The verb “get” can be informal. Consider replacing it with a form of “to be”.
Context: ...unt handler's metadata can define which get passed to the state handler when an accoun...

(GET_USED_ARE_USED)


[uncategorized] ~170-~170: Loose punctuation mark.
Context: ...store the account needs. begin_tx, commit_tx, rollback_tx and `discard...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~173-~173: Possible missing comma found.
Context: ...ch message request. It is opaque to the hypervisor except that the first bit of the first ...

(AI_HYDRA_LEO_MISSING_COMMA)


[typographical] ~175-~175: Consider adding a comma.
Context: ... the state token. If the high bit is set then the state token is volatile, and if...

(IF_THEN_COMMA)


[style] ~179-~179: For conciseness, try rephrasing this sentence.
Context: ...rdinate around the usage of this token. It is possible that state modules expose methods for creating new **state tokens...

(MAY_MIGHT_BE)


[style] ~183-~183: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... describes a message handler's behavior with respect to state and side effects. It is an enum v...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)


[uncategorized] ~186-~186: Loose punctuation mark.
Context: ... both read and write state. * readonly: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...re expected to only read state. * pure: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~299-~299: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... ### Error Codes & Handling All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)


[uncategorized] ~353-~353: Possible missing comma found.
Context: ...tack up to the last recoverable message handler if there is one. When unwinding, the hy...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~357-~357: Consider an alternative adverb to strengthen your wording.
Context: ...ed transactions, these transactions are properly cleaned up. Any state token that was us...

(PROPERLY_THOROUGHLY)

Markdownlint
docs/rfc/rfc-003-crosslang.md

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


161-161: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


162-162: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


163-163: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


185-185: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


265-265: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


266-266: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


267-267: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


268-268: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


276-276: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


277-277: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


278-278: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


302-302: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


259-259: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


276-276: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


289-289: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


304-304: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Comment on lines 299 to 366
All invoke and handler methods return a 32-bit unsigned integer error code.
If the error code is zero, then the operation was successful.

#### System-Level Error Codes
The following non-zero error codes are defined at the system level:
* 1: out of gas
* 2: fatal execution error (when an unexpected, likely non-deterministic fatal error occurs)
* 3: account not found
* 4: message handler not found
* 5: invalid state access (when volatility rules are violated)
* 6: unauthorized caller address (when the caller address impersonation is not authorized)
* 7: invalid handler (when hypervisor or vm-level rules are violated by the handler implementation)
* 8: unknown handler error (when handler execution failed for an unknown, but likely deterministic reason)

Error codes through 255 are reserved for system-level errors and should only be returned by the hypervisor or
a virtual machine implementation.

#### Handler-level Error Codes

Any error code above 255 is interpreted as an error returned by a handler itself and is to be interpreted by the caller
based on the message handler's specification.

It is an error for a handler implementation to return a system-level error code, and if such a code is received it
will be translated to error code 7 (invalid handler).
If a handler implementation wants to simply propagate a system-level error code that it receives,
it should wrap it with a different error code.
Developer SDKs should handle this wrapping automatically.

#### Error Data

If additional data is included in the error,
it is generally expected that this would be referenced by **output data pointer 1**,
although message handlers are free to use **output data pointer 2** if necessary as well.
System-level errors, if they do include additional data, will encode it as a string in **output data pointer 1**
and will limit such messages to a maximum of 255 bytes.

#### Errors and State Transactions

Whenever a volatile message handler returns an error, no side effects can occur.
If any side effects were applied to state before the error occurred, these must be rolled back.
The hypervisor manages this using the `begin_tx`, `commit_tx`, and `rollback_tx` on the **state handler**.

Before any call to a volatile method,
the hypervisor will call `begin_tx` with the state token passed in the message request.
If the method returns the core `0` for success, then the hypervisor will call `commit_tx` with the state token.
If the method returns an error code, then the hypervisor will call `rollback_tx` with the state token
and return the error to the caller.

This ensures state consistency in the presence of errors.

#### Unwinding Errors and Discarding State Tokens

Error codes 1 (out of gas) and 2 (fatal execution error) are considered unwinding errors.
If one of these errors is returned, the hypervisor will halt execution of the calling message handler
and unwind the call stack up to the last recoverable message handler if there is one.
When unwinding, the hypervisor will call the `discard_cleanup` method of the **state handler**
with each _new_ state token that was introduced in the unwound call stack.
This ensures that if message handlers used new state tokens to create nested transactions,
these transactions are properly cleaned up.
Any state token that was used in the call stack before the unwinding target will not be cleaned up.

For out-of-gas errors (code 1), the hypervisor will unwind up to the caller that set the gas limit.
For fatal execution errors (code 2), the hypervisor will unwind up to the external caller that called the hypervisor, likely
resulting in process termination.
A fatal execution error should only be returned when it is expected that the error is non-deterministic and unrecoverable.
An example of such an error would be running out of disk space or losing network connectivity.
If there is an execution error that is likely to be deterministic, such as Wasm code failing to execute, then
the virtual machine should return error code 8 (unknown handler error), which is not an unwinding error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Review Error Handling Section for Clarity and Grammar

The error handling section is critical for understanding how errors are managed in the system. However, there are several grammatical issues and potential clarifications needed.

Consider the following improvements for clarity and grammatical correctness:

### Error Codes & Handling

All invoke and handler methods return a 32-bit unsigned integer error code. If the error code is zero, then the operation was successful.

#### System-Level Error Codes

The following non-zero error codes are defined at the system level:
* `1`: out of gas
* `2`: fatal execution error (when an unexpected, likely non-deterministic fatal error occurs)
* `3`: account not found
* `4`: message handler not found
* `5`: invalid state access (when volatility rules are violated)
* `6`: unauthorized caller address (when the caller address impersonation is not authorized)
* `7`: invalid handler (when hypervisor or vm-level rules are violated by the handler implementation)
* `8`: unknown handler error (when handler execution failed for an unknown, but likely deterministic reason)

Error codes through 255 are reserved for system-level errors and should only be returned by the hypervisor or a virtual machine implementation.

#### Handler-level Error Codes

Any error code above 255 is interpreted as an error returned by a handler itself and is to be interpreted by the caller based on the message handler's specification.

It is an error for a handler implementation to return a system-level error code, and if such a code is received, it will be translated to error code `7` (invalid handler). If a handler implementation wants to simply propagate a system-level error code that it receives, it should wrap it with a different error code. Developer SDKs should handle this wrapping automatically.

#### Error Data

If additional data is included in the error, it is generally expected that this would be referenced by **output data pointer 1**, although message handlers are free to use **output data pointer 2** if necessary as well. System-level errors, if they do include additional data, will encode it as a string in **output data pointer 1** and will limit such messages to a maximum of 255 bytes.

#### Errors and State Transactions

Whenever a volatile message handler returns an error, no side effects can occur. If any side effects were applied to state before the error occurred, these must be rolled back. The hypervisor manages this using the `begin_tx`, `commit_tx`, and `rollback_tx` on the **state handler**.

Before any call to a volatile method, the hypervisor will call `begin_tx` with the state token passed in the message request. If the method returns the core `0` for success, then the hypervisor will call `commit_tx` with the state token. If the method returns an error code, then the hypervisor will call `rollback_tx` with the state token and return the error to the caller.

This ensures state consistency in the presence of errors.

#### Unwinding Errors and Discarding State Tokens

Error codes `1` (out of gas) and `2` (fatal execution error) are considered unwinding errors. If one of these errors is returned, the hypervisor will halt execution of the calling message handler and unwind the call stack up to the last recoverable message handler if there is one. When unwinding, the hypervisor will call the `discard_cleanup` method of the **state handler** with each _new_ state token that was introduced in the unwound call stack. This ensures that if message handlers used new state tokens to create nested transactions, these transactions are properly cleaned up. Any state token that was used in the call stack before the unwinding target will not be cleaned up.

For out-of-gas errors (code `1`), the hypervisor will unwind up to the caller that set the gas limit. For fatal execution errors (code `2`), the hypervisor will unwind up to the external caller that called the hypervisor, likely resulting in process termination. A fatal execution error should only be returned when it is expected that the error is non-deterministic and unrecoverable. An example of such an error would be running out of disk space or losing network connectivity. If there is an execution error that is likely to be deterministic, such as Wasm code failing to execute, then the virtual machine should return error code `8` (unknown handler error), which is not an unwinding error.
Tools
LanguageTool

[uncategorized] ~299-~299: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ... ### Error Codes & Handling All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)


[uncategorized] ~353-~353: Possible missing comma found.
Context: ...tack up to the last recoverable message handler if there is one. When unwinding, the hy...

(AI_HYDRA_LEO_MISSING_COMMA)


[style] ~357-~357: Consider an alternative adverb to strengthen your wording.
Context: ...ed transactions, these transactions are properly cleaned up. Any state token that was us...

(PROPERLY_THOROUGHLY)

Markdownlint

302-302: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)


304-304: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range and nitpick comments (6)
docs/rfc/rfc-003-crosslang.md (6)

170-171: Minor punctuation improvement.

The explanation of the purpose of the new state handler methods is clear and concise. However, there is a loose punctuation mark before the method names.

Apply this diff to fix the punctuation:

-`begin_tx`, `commit_tx`, `rollback_tx` and `discard_cleanup` are used internally by the hypervisor for error handling.
+`begin_tx`, `commit_tx`, `rollback_tx`, and `discard_cleanup` are used internally by the hypervisor for error handling.
Tools
LanguageTool

[uncategorized] ~170-~170: Loose punctuation mark.
Context: ...store the account needs. begin_tx, commit_tx, rollback_tx and `discard...

(UNLIKELY_OPENING_PUNCTUATION)


172-177: Minor punctuation improvement.

The explanation of the state token and its volatility is clear and informative. However, consider adding a comma to improve readability.

Apply this diff to add a comma:

-If the high bit is set then the state token is **volatile**, and if it is unset, it is **readonly**.
+If the high bit is set, then the state token is **volatile**, and if it is unset, it is **readonly**.
Tools
LanguageTool

[typographical] ~175-~175: Consider adding a comma.
Context: ... the state token. If the high bit is set then the state token is volatile, and if...

(IF_THEN_COMMA)


185-190: Minor punctuation improvements.

The definition of the volatility enum and the behaviors of each value are clear and concise. However, there are a few loose punctuation marks that can be removed to improve readability.

Apply this diff to remove the loose punctuation marks:

-* `volatile`: the handler can have side effects and send `volatile`, `readonly` or `pure` messages to other accounts. Such handlers are expected to both read and write state.
-* `readonly`: the handler cannot cause effects side effects and can only send `readonly` or `pure` messages to other accounts. Such handlers are expected to only read state.
-* `pure`: the handler cannot cause any side effects and can only call other pure handlers. Such handlers are expected to neither read nor write state.
+`volatile`: the handler can have side effects and send `volatile`, `readonly` or `pure` messages to other accounts. Such handlers are expected to both read and write state.
+`readonly`: the handler cannot cause effects side effects and can only send `readonly` or `pure` messages to other accounts. Such handlers are expected to only read state.
+`pure`: the handler cannot cause any side effects and can only call other pure handlers. Such handlers are expected to neither read nor write state.
Tools
LanguageTool

[uncategorized] ~186-~186: Loose punctuation mark.
Context: ... both read and write state. * readonly: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)


[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...re expected to only read state. * pure: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)

Markdownlint

185-185: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


258-272: Minor list style improvement.

The specification of the message packet header layout is detailed and includes all the necessary information for routing and processing messages. Reserving the remaining bytes for future use and requiring them to be zeroed is a good practice for ensuring forward compatibility.

However, the list style is inconsistent with the rest of the document.

Apply this diff to change the list style to asterisks:

-- **message name**: a 128-byte array with the first byte indicating the length of the message name
-- **self-address**: a 64-byte array with the first byte indicating the length of the address
-- **caller-address**: a 64-byte array with the first byte indicating the length of the address
-- **context token**: a 32-byte array
-- **state token**: a 32-byte array
-- **message name hash**: the first 8-bytes of the SHA256 hash of the message name, which can be used for simplified routing
-- **gas limit**: an unsigned 64-bit integer
-- **gas consumed**: an unsigned 64-bit integer
-- **input data pointer 1**: 16 bytes, see below for the **data pointer** spec
-- **input data pointer 2**: 16 bytes
-- **output data pointer 1**: 16 bytes
-- **output data pointer 2**: 16 bytes
-- remaining bytes: reserved for future use, should be zeroed when a packet is initialized
+* **message name**: a 128-byte array with the first byte indicating the length of the message name
+* **self-address**: a 64-byte array with the first byte indicating the length of the address
+* **caller-address**: a 64-byte array with the first byte indicating the length of the address
+* **context token**: a 32-byte array
+* **state token**: a 32-byte array
+* **message name hash**: the first 8-bytes of the SHA256 hash of the message name, which can be used for simplified routing
+* **gas limit**: an unsigned 64-bit integer
+* **gas consumed**: an unsigned 64-bit integer
+* **input data pointer 1**: 16 bytes, see below for the **data pointer** spec
+* **input data pointer 2**: 16 bytes
+* **output data pointer 1**: 16 bytes
+* **output data pointer 2**: 16 bytes
+* remaining bytes: reserved for future use, should be zeroed when a packet is initialized
Tools
Markdownlint

259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


265-265: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


266-266: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


267-267: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


268-268: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


259-259: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


275-287: Minor list style improvement.

The specification of the data pointer structure is clear and well-defined. The behavior of the data pointer based on the value of the native pointer ensures efficient data passing between different environments.

However, the list style is inconsistent with the rest of the document.

Apply this diff to change the list style to asterisks:

-- **native pointer**: 8-bytes, a pointer to a separate buffer in the native environment or zero
-- **length**: unsigned 32-bit integer
-- **capacity or offset**: unsigned 32-bit integer
+* **native pointer**: 8-bytes, a pointer to a separate buffer in the native environment or zero
+* **length**: unsigned 32-bit integer
+* **capacity or offset**: unsigned 32-bit integer
Tools
Markdownlint

276-276: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


277-277: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


278-278: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)


276-276: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)


288-296: Minor list style improvement.

The procedure for passing a packet from one environment to another is clear and well-defined. The steps ensure that the necessary data is copied and allocated correctly in the new environment, and the handling of input and output data pointers is efficient and accurate.

However, the list style is inconsistent with the rest of the document.

Apply this diff to change the list style to numbers:

-1. allocate the packet in the new environment
-2. copy all 512-bytes of the **message packet** header from the source packet to the target packet
-3. if input data pointers point to additional buffers, then allocate these buffers in the new environment
-and update the **data pointers** in the target packet to point to the new buffers

<details>
<summary>Tools</summary>

<details>
<summary>Markdownlint</summary><blockquote>

289-289: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

</blockquote></details>

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>Review details</summary>

**Configuration used: .coderabbit.yml**
**Review profile: CHILL**

<details>
<summary>Commits</summary>

Files that changed from the base of the PR and between 6ee34677a6dcb47fea44a28285072dd5080d5a70 and 1d4075f5399b4d7ea3ee60a44ae2f3af3145a597.

</details>


<details>
<summary>Files selected for processing (1)</summary>

* docs/rfc/rfc-003-crosslang.md (3 hunks)

</details>












<details>
<summary>Additional context used</summary>

<details>
<summary>Path-based instructions (1)</summary><blockquote>

<details>
<summary>docs/rfc/rfc-003-crosslang.md (1)</summary>

Pattern `**/*.md`: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"


</details>

</blockquote></details>

<details>
<summary>LanguageTool</summary><blockquote>

<details>
<summary>docs/rfc/rfc-003-crosslang.md</summary><blockquote>

[style] ~167-~167: The verb “get” can be informal. Consider replacing it with a form of “to be”.
Context: ...unt handler's metadata can define which get passed to the **state handler** when an accoun...

(GET_USED_ARE_USED)

---

[uncategorized] ~170-~170: Loose punctuation mark.
Context: ...store the **account** needs.  `begin_tx`, `commit_tx`, `rollback_tx` and `discard...

(UNLIKELY_OPENING_PUNCTUATION)

---

[typographical] ~175-~175: Consider adding a comma.
Context: ... the state token. If the high bit is set then the state token is **volatile**, and if...

(IF_THEN_COMMA)

---

[style] ~179-~179: For conciseness, try rephrasing this sentence.
Context: ...rdinate around the usage of this token. It is possible that state modules expose methods for creating new **state tokens...

(MAY_MIGHT_BE)

---

[style] ~183-~183: ‘with respect to’ might be wordy. Consider a shorter alternative.
Context: ... describes a message handler's behavior with respect to state and side effects. It is an enum v...

(EN_WORDINESS_PREMIUM_WITH_RESPECT_TO)

---

[uncategorized] ~186-~186: Loose punctuation mark.
Context: ... both read and write state. * `readonly`: the handler cannot cause effects side e...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~187-~187: Loose punctuation mark.
Context: ...re expected to only read state. * `pure`: the handler cannot cause any side effec...

(UNLIKELY_OPENING_PUNCTUATION)

---

[uncategorized] ~299-~299: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...  ### Error Codes & Handling  All invoke and handler methods return a 32-bit unsigne...

(COMMA_COMPOUND_SENTENCE_2)

---

[style] ~361-~361: Consider an alternative adverb to strengthen your wording.
Context: ...ed transactions, these transactions are properly cleaned up. Any state token that was us...

(PROPERLY_THOROUGHLY)

</blockquote></details>

</blockquote></details>
<details>
<summary>Markdownlint</summary><blockquote>

<details>
<summary>docs/rfc/rfc-003-crosslang.md</summary><blockquote>

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

161-161: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

162-162: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

163-163: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

185-185: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

---

259-259: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

260-260: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

261-261: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

262-262: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

263-263: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

264-264: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

265-265: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

266-266: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

267-267: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

268-268: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

269-269: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

270-270: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

271-271: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

276-276: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

277-277: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

278-278: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

302-302: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines

(MD022, blanks-around-headings)

---

259-259: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

---

276-276: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

---

289-289: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

---

304-304: null
Lists should be surrounded by blank lines

(MD032, blanks-around-lists)

</blockquote></details>

</blockquote></details>

</details>
<details>
<summary>Additional comments not posted (3)</summary><blockquote>

<details>
<summary>docs/rfc/rfc-003-crosslang.md (3)</summary><blockquote>

`160-164`: **LGTM!**

The addition of the `begin_tx`, `commit_tx`, `rollback_tx`, and `discard_cleanup` methods to the state handler interface is a good change. These methods are used by the hypervisor to manage nested transactions and handle errors, ensuring state consistency in the presence of errors.

<details>
<summary>Tools</summary>

<details>
<summary>Markdownlint</summary><blockquote>

160-160: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

161-161: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

162-162: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

---

163-163: Expected: asterisk; Actual: dash
Unordered list style

(MD004, ul-style)

</blockquote></details>

</details>

---

`193-196`: **LGTM!**

The rules for calling handlers based on their volatility are well-defined and ensure that the volatility of the handlers is respected. The exception for readonly handlers acquiring a new volatile state token for simulations is a useful addition.

---

`251-255`: **LGTM!**

The definition of the minimum size of message packets and the allowance for larger packets based on message handler needs is clear and reasonable. Delegating the responsibility of describing the precise utilization of the message packet to each message handler specification is a good approach.

</blockquote></details>

</blockquote></details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Copy link
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the Stale label Oct 17, 2024
@github-actions github-actions bot closed this Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants