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

Consider offering a chain-specific memo config #3811

Closed
5 tasks
rootulp opened this issue Jan 24, 2024 · 1 comment · Fixed by #3863
Closed
5 tasks

Consider offering a chain-specific memo config #3811

rootulp opened this issue Jan 24, 2024 · 1 comment · Fixed by #3863
Assignees
Milestone

Comments

@rootulp
Copy link
Contributor

rootulp commented Jan 24, 2024

Summary

Consider offering a chain-specific memo config

Problem Definition

Celestia is considering decreasing auth.MaxMemoCharacters from 256 to 16 (see celestiaorg/CIPs#44). While working on that proposal, I observed that a number of the most frequent memos are relayer memos of the form:

mzonder | hermes 1.7.4+ab73266 (https://hermes.informal.systems)` 

The above memo is 64 characters so if the linked Celestia improvement proposal (CIP) is approved, memos of this format will be considered invalid. Put another way, Hermes relayers that are relaying transactions on Celestia will stop relaying.

Proposal

I noticed that Hermes config allows users to specify a portion of the memo. Ref:

# Specify a string that Hermes will use as a memo for each transaction it submits
# to this chain. The string is limited to 50 characters. Default: '' (empty).
# Note: Hermes will append to the string defined here additional
# operational debugging information, e.g., relayer build version.
memo_prefix = ''

However, this option doesn't enable relayers to override the entire memo. Since the 16 character memo limitation is Celestia-specific, proposal to modify the mechanics around this chain-specific config.

Option A (non-breaking)

I think it would be non-breaking to add a new chain-specific config for the memo field.

  • If memo is provided, it sets the entire memo. The value specified by memo_prefix is ignored. No relayer build information is suffixed to this memo.
  • If memo isn't provided, the current behavior is respected.

This approach would enable relayers to continue operating on Celestia.

Option B (breaking)

If maintainers are open to breaking existing behavior and would like to preserve the diagnostic information in the memo, consider reducing the suffix to just the version + commit hash portion. E.g. 1.7.4+ab73266 which is only 13 characters.

If users don't specify a memo_prefix, default to the version + commit hash portion.
If users do specify a memo_prefix, don't suffix with version + commit hash portion.

If this option is taken, it's probably worth renaming memo_prefix => memo as well.

Acceptance Criteria

TBD. Opening an issue to start the discussion.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@github-project-automation github-project-automation bot moved this to 🩹 Triage in Hermes Jan 24, 2024
@romac romac added this to the v1.8.1 milestone Jan 24, 2024
@romac
Copy link
Member

romac commented Feb 8, 2024

Thanks for the heads-up, sounds like a good idea! We'll look into it for the next release.

@github-project-automation github-project-automation bot moved this from 🩹 Triage to ✅ Done in Hermes Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants