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 FeeRecipientModule and SendTipToProposer params to update fee and tip handling #NTRN-424 #1

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

joldie777
Copy link

@joldie777 joldie777 commented Jan 9, 2025

@joldie777 joldie777 changed the title add FeeRecipientModule and SendTipToProposer params to update fee and tip handling feat: add FeeRecipientModule and SendTipToProposer params to update fee and tip handling #NTRN-424 Jan 9, 2025
@joldie777 joldie777 force-pushed the feat/update-fee-and-tip-handling branch from e14b726 to ca5415d Compare January 13, 2025 02:32
@joldie777 joldie777 force-pushed the feat/update-fee-and-tip-handling branch from ca5415d to d982e91 Compare January 13, 2025 02:45
) (string, error) {
var err error

tipPayee := feeRecipientModule
Copy link

Choose a reason for hiding this comment

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

In this code, the tipPayee variable takes on different formats depending on the if condition. Specifically, feeRecipientModule is a human-readable name of the module, while proposer.String() represents a Bech32 address, such as neutron1blablabla.

Currently, the return value of tipPayee is used only for broadcasting an event. However, future use cases might arise where users expect a consistent format, regardless of the params.SendTipToProposer condition. Returning two semantically different values under the same variable name can lead to confusion and potential bugs. This behavior should be fixed to ensure tipPayee always has a consistent and predictable format.

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