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: add configurable limits for receiver and ICS-20 memo fields #1377

Merged
merged 11 commits into from
Jan 26, 2024

Conversation

jtieri
Copy link
Member

@jtieri jtieri commented Jan 9, 2024

This PR adds two new additional fields to the relayer's global config, ICS20MemoLimit & MaxReceiverSize. These new values can be used to fine tune the relayer to ignore packets whose receiver field or ics-20 memo field exceed the configured limit. By default ICS20MemoLimit is configured to zero which indicates that no limit is set, MaxReceiverSize is configured to 128 which indicates that receiver fields greater than 128 bytes should be ignored.

agouin and others added 4 commits January 8, 2024 16:36
This change introduces new configuration, MaxReceiverSize & ICS20MemoLimit, in the relayer, path processor, and the global config. It updates the existing functions and methods to utilize this new configuration property for managing and validating receiver and memo sizes in IBC related tasks.
Added a new function WriteConfig in relayertest/system.go for testing. Also, updated method names in relayer.go to start with an uppercase letter for public visibility. Besides, upgraded Docker image version in localhost_client_test.go from v8.0.0-beta.1 to v8.0.0.
Created the Memo and Receiver Limit test within the interchain test suite. This test evaluates the functionality of IBC transfers with a memo field length exceeding configured limit, as well as receiver field exceeding the limit. It also evaluates a successful IBC transfer for completeness.
@jtieri jtieri mentioned this pull request Jan 9, 2024
Adjusted the test case 'TestMemoAndReceiverLimit' to focus more on the functionality of sending transfers with a memo and receiver limit configured. This reduces the complexity of the test case, and it now focuses more closely on the actual purpose of the method. The general flow of the test has been maintained, but some unnecessary operations have been removed.
…ments

This commit removes various debug print statements from the code, making it cleaner and easier to read. Additionally, it updates the way the 'transfertypes.ModuleCdc.Unmarshal' function is used to 'transfertypes.ModuleCdc.UnmarshalJSON', simplifying the overall structure of the code.
The test for the memo receiver limit now supports four test users instead of two, allowing for more complex transfer scenarios. This change requires increased checks for account balances, as well as additional IBC transfers. This expansion helps ensure more comprehensive testing coverage and overall system validation.
@jtieri jtieri marked this pull request as ready for review January 9, 2024 19:30
@jtieri jtieri enabled auto-merge (squash) January 14, 2024 01:25
@jtieri jtieri merged commit 5ab55c0 into main Jan 26, 2024
18 of 19 checks passed
@jtieri jtieri deleted the justin/memo-receiver-limit branch January 26, 2024 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants