Create ics27-multiplexed-requirements.md #6281
Conversation
womensrights
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei,
damiannolan,
chatton,
DimitrisJim and
srdtrk
as code owners
WalkthroughWalkthroughThe Changes
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Out of diff range and nitpick comments (7)
docs/requirements/ics27-multiplexed-requirements.md (7)
11-11: Consider adding a comma after "Account Flexibility" for better readability.
13-13: Consider using "time-consuming" with a hyphen for correct spelling.
19-19: Use "add-on" with a hyphen in this context for correct spelling.
26-26: Use "per-user" with a hyphen when used as an adjective.
56-56: Consider adding a comma after "requirements" for better readability.
83-83: Use "per-user" with a hyphen when used as an adjective.
85-85: Ensure consistency in spelling; use either "acknowledgement" or "acknowledgment" throughout the document.
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (7)
docs/requirements/ics27-multiplexed-requirements.md (7)
Near line 11: Possible missing comma found.
Context: ... are listed: - Account Flexibility - Currently host accounts can only be of type `Base...
Near line 13: This word is normally spelled with a hyphen.
Context: ...uts and subsequent channel closures are time consuming and difficult to manage - *Scalability...
Near line 19: This noun is usually spelled with a hyphen.
Context: ...iddleware solves this problem but is an add on to the application rather than a defaul...
Near line 26: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...nt management for accounts created on a per user basis - To streamline workflows using t...
Near line 56: Possible missing comma found.
Context: ...desirable, it is out of scope for these requirements as a solution is applicable to applicat...
Near line 83: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...implementing additional middleware on a per user basis | ------------ | ------ | | 3.06 ...
Near line 85: Do not mix variants of the same word (‘acknowledgement’ and ‘acknowledgment’) within a single text.
Context: ...in account and return the result in the acknowledgement | ------------ | ------ | ### 4 - Ho...
GitHub Check Runs (1)
lint failure (3)
docs/requirements/ics27-multiplexed-requirements.md: [failure] 53-53: Headings should be surrounded by blank lines
docs/requirements/ics27-multiplexed-requirements.md:53 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "# Functional requirements"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
docs/requirements/ics27-multiplexed-requirements.md: [failure] 54-54: Headings should be surrounded by blank lines
docs/requirements/ics27-multiplexed-requirements.md:54 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Above] [Context: "## Assumptions and dependencies"] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md022.md
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
There was a problem hiding this comment.
Actionable comments posted: 9
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (7)
docs/requirements/ics27-multiplexed-requirements.md (7)
Near line 13: This word is normally spelled with a hyphen.
Context: ...uts and subsequent channel closures are time consuming and difficult to manage - *Scalability...
Near line 17: Possible missing comma found.
Context: ...nsfer could succeed and the ICA message fail resulting in an incomplete workflow. ...
Near line 19: This noun is usually spelled with a hyphen.
Context: ...iddleware solves this problem but is an add on to the application rather than a defaul...
Near line 26: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...nt management for accounts created on a per user basis - To streamline workflows using t...
Near line 57: Possible missing comma found.
Context: ...desirable, it is out of scope for these requirements as a solution is applicable to applicat...
Near line 84: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...implementing additional middleware on a per user basis | ------------ | ------ | | 3.06 ...
Near line 86: Do not mix variants of the same word (‘acknowledgement’ and ‘acknowledgment’) within a single text.
Context: ...in account and return the result in the acknowledgement | ------------ | ------ | ### 4 - Ho...
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 24
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (7)
docs/requirements/ics27-multiplexed-requirements.md (7)
Near line 11: Possible missing comma found.
Context: ... are listed: - Account Flexibility - Currently host accounts can only be of type `Base...
Near line 13: This word is normally spelled with a hyphen.
Context: ...uts and subsequent channel closures are time consuming and difficult to manage - *Scalability...
Near line 19: This noun is usually spelled with a hyphen.
Context: ...iddleware solves this problem but is an add on to the application rather than a defaul...
Near line 26: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...nt management for accounts created on a per user basis - To streamline workflows using t...
Near line 57: Possible missing comma found.
Context: ...desirable, it is out of scope for these requirements as a solution is applicable to applicat...
Near line 84: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...implementing additional middleware on a per user basis | ------------ | ------ | | 3.06 ...
Near line 86: Do not mix variants of the same word (‘acknowledgement’ and ‘acknowledgment’) within a single text.
Context: ...in account and return the result in the acknowledgement | ------------ | ------ | ### 4 - Ho...
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
| | 3.04 | The controller of the interchain account should be able to receive information about the balance of the interchain account in the acknowledgment after a transaction was executed by the host | ------------ | ------ | | ||
| | 3.05 | The user of the controller should be able to receive all the information contained in the acknowledgment without implementing additional middleware on a per user basis | ------------ | ------ | | ||
| | 3.06 | Callbacks on the packet lifecycle should be supported by default | ------------ | ------ | | ||
| | 3.07 | A user can perform module safe queries through a host chain account and return the result in the acknowledgement | ------------ | ------ | |
There was a problem hiding this comment.
Ensure consistency in the spelling of "acknowledgment".
- in the acknowledgement
+ in the acknowledgmentCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| | 3.07 | A user can perform module safe queries through a host chain account and return the result in the acknowledgement | ------------ | ------ | | |
| | 3.07 | A user can perform module safe queries through a host chain account and return the result in the acknowledgment | ------------ | ------ | |
There was a problem hiding this comment.
Actionable comments posted: 3
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (3)
docs/requirements/ics27-multiplexed-requirements.md (3)
Near line 17: Possible missing comma found.
Context: ...nsfer could succeed and the ICA message fail resulting in an incomplete workflow. ...
Near line 26: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...nt management for accounts created on a per user basis - To streamline workflows using t...
Near line 84: In this context, “per-user” forms an adjective and is spelled with a hyphen.
Context: ...implementing additional middleware on a per user basis | ------------ | ------ | | 3.06 ...
GitHub Check Runs (1)
lint failure (2)
docs/requirements/ics27-multiplexed-requirements.md: [failure] 26-26: Lists should be surrounded by blank lines
docs/requirements/ics27-multiplexed-requirements.md:26 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- To facilitate performant cro..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md032.md
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Near line 11: Possible missing comma found.
Context: ... are listed: - Account Flexibility - Currently host accounts can only be of type `Base...
GitHub Check Runs (1)
lint failure (3)
docs/requirements/ics27-multiplexed-requirements.md: [failure] 26-26: Lists should be surrounded by blank lines
docs/requirements/ics27-multiplexed-requirements.md:26 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- To facilitate performant cro..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md032.md
docs/requirements/ics27-multiplexed-requirements.md: [failure] 73-73: Table column count
docs/requirements/ics27-multiplexed-requirements.md:73:145 MD056/table-column-count Table column count [Expected: 4; Actual: 6; Too many cells, extra data will be missing] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md056.md
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 13
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
LanguageTool (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Near line 11: Possible missing comma found.
Context: ... are listed: - Account Flexibility - Currently host accounts can only be of type `Base...
GitHub Check Runs (1)
lint failure (2)
docs/requirements/ics27-multiplexed-requirements.md: [failure] 26-26: Lists should be surrounded by blank lines
docs/requirements/ics27-multiplexed-requirements.md:26 MD032/blanks-around-lists Lists should be surrounded by blank lines [Context: "- To streamline workflows usin..."] https://github.com/DavidAnson/markdownlint/blob/v0.34.0/doc/md032.md
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
|
|
||
| The current pain points for existing ICA users are listed: | ||
|
|
||
| - *Account Flexibility* - Currently host accounts can only be of type `BaseAccount` |
There was a problem hiding this comment.
Consider adding a comma after "listed" for better readability.
- The current pain points for existing ICA users are listed:
+ The current pain points for existing ICA users are listed:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| - *Account Flexibility* - Currently host accounts can only be of type `BaseAccount` |
| Enable different account types, to be controlled remotely | ||
| - To streamline workflows using token transfer and interchain accounts in combination |
There was a problem hiding this comment.
Lists should be surrounded by blank lines for proper markdown formatting.
- Enable different account types, to be controlled remotely
- To streamline workflows using token transfer and interchain accounts in combination
+
+ Enable different account types, to be controlled remotely
+ - To streamline workflows using token transfer and interchain accounts in combination
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| Enable different account types, to be controlled remotely | |
| - To streamline workflows using token transfer and interchain accounts in combination | |
| Enable different account types, to be controlled remotely | |
| - To streamline workflows using token transfer and interchain accounts in combination | |
There was a problem hiding this comment.
Actionable comments posted: 12
Review Details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional Context Used
Path-based Instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
| - Enable different account types, to be controlled remotely | ||
| - To streamline workflows using token transfer and interchain accounts in combination |
There was a problem hiding this comment.
Lists should be surrounded by blank lines for proper markdown formatting.
+
- Enable different account types, to be controlled remotely
- To streamline workflows using token transfer and interchain accounts in combination
+ Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| - Enable different account types, to be controlled remotely | |
| - To streamline workflows using token transfer and interchain accounts in combination | |
| - Enable different account types, to be controlled remotely | |
| - To streamline workflows using token transfer and interchain accounts in combination | |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (13)
docs/requirements/ics27-multiplexed-requirements.md (13)
5-5: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- These requirements aim to alleviate current pain points and enhance the usability of the feature. + These requirements aim to alleviate current pain points and enhance the usability of the feature.
13-13: Add a hyphen to "time-consuming".Add a hyphen to form the correct adjective.
- Timeouts and subsequent channel closures are time consuming and difficult to manage + Timeouts and subsequent channel closures are time-consuming and difficult to manage
17-17: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- The transfer could succeed and the ICA message fail, resulting in an incomplete workflow. + The transfer could succeed and the ICA message fail, resulting in an incomplete workflow.
19-19: Add a hyphen to "add-on".Add a hyphen to form the correct adjective.
- the callbacks middleware solves this problem but is an add on to the application rather than a default + the callbacks middleware solves this problem but is an add-on to the application rather than a default
21-21: Consider changing "Whitelisiting" to "Whitelisting".Correct the spelling error for clarity.
- *Whitelisiting messages* - this devex is more complicated than having a blacklist + *Whitelisting messages* - this devex is more complicated than having a blacklist
47-47: Correct the spelling of "principle" to "principal".Correct the spelling error for clarity.
- Nolus enables users to borrow assets and use the inflation from staking rewards to repay the interest on the principle. + Nolus enables users to borrow assets and use the inflation from staking rewards to repay the interest on the principal.
52-52: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- Nomos enable NFTs to be minted on Injective on host accounts controlled from the Xion chain, providing a single interface for users, enabling chain abstraction. + Nomos enable NFTs to be minted on Injective on host accounts controlled from the Xion chain, providing a single interface for users, enabling chain abstraction.
57-57: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- Although having atomic transfer plus action workflows with Interchain Accounts is desirable, it is out of scope for these requirements, as a solution is applicable to applications beyond Interchain Accounts alone. + Although having atomic transfer plus action workflows with Interchain Accounts is desirable, it is out of scope for these requirements, as a solution is applicable to applications beyond Interchain Accounts alone.
67-67: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- | 1.01 | The host chain should accept all message types by default and maintain a blacklist of message types it does not permit | ------------ | ------ | + | 1.01 | The host chain should accept all message types by default and maintain a blacklist of message types it does not permit | ------------ | ------ |
73-73: Correct the table formatting by ensuring the correct number of cells per row.Ensure consistency in table formatting for clarity.
- | 2.01 | The controller of the interchain account must have authority over the account on the host chain to execute messages | -- | ----------- | + | 2.01 | The controller of the interchain account must have authority over the account on the host chain to execute messages | ------------ | ------ |
84-84: Add a hyphen to "per-user" to form the correct adjective.Ensure consistency in the use of compound adjectives.
- without implementing additional middleware on a per user basis + without implementing additional middleware on a per-user basis
86-86: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- | 3.07 | A user can perform module safe queries through a host chain account and return the result in the acknowledgment | ------------ | ------ | + | 3.07 | A user can perform module safe queries through a host chain account and return the result in the acknowledgment | ------------ | ------ |
100-100: Remove trailing space.Remove the trailing space at the end of the line for cleaner formatting.
- | 5.01 | The number of packet round trips to register an account, load the account with tokens and execute messages on the account should be minimised | ------------ | ------ | + | 5.01 | The number of packet round trips to register an account, load the account with tokens and execute messages on the account should be minimised | ------------ | ------ |
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- docs/requirements/ics27-multiplexed-requirements.md (1 hunks)
Additional context used
Path-based instructions (1)
docs/requirements/ics27-multiplexed-requirements.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
|
Should this be merged or closed? Or migrated elsewhere for the time being. I'm also fine with leaving it sit. Just doing some housekeeping 🧹 |
|
I think it should be merged, can always be changed later but given its a draft think its fine |
There was a problem hiding this comment.
Agree with @DimitrisJim. Feel free to merge if you want. Could maybe add a notice at the top of document that it is a draft or something like that.
Description
Requirements for to enable multiplexed interchain accounts - many controllers controlling many host accounts through a single channel
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.Summary by CodeRabbit