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

rebate integration tests #1152

Merged
merged 11 commits into from
Mar 21, 2024
Merged

rebate integration tests #1152

merged 11 commits into from
Mar 21, 2024

Conversation

sampocs
Copy link
Collaborator

@sampocs sampocs commented Mar 20, 2024

Context

Setup integration test scripts for rebate testing. There were also a few small bugs fixed in this process.

See dockernet/scripts/community-pool-staking/README.md

Chain Changes

  • Removed check that price was non-empty before sending transfer
  • Fixed denom type in fund community pool message
  • Added expiration to authz message

Off-chain Changes

  • Added treasury to dydx's host zone and to balances.log
  • Added rebate.sh script to register a rebate
  • Added trade.sh script to execute authz trade

Copy link
Contributor

@riley-stride riley-stride left a comment

Choose a reason for hiding this comment

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

chain code changes lgtm - nice catches!

@sampocs sampocs merged commit 2f8e27d into rebate Mar 21, 2024
1 check passed
Copy link
Contributor

@ethan-stride ethan-stride left a comment

Choose a reason for hiding this comment

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

LGTM, mainly just looked at the on chain changes, didn't scour the integration tests

@@ -67,7 +67,7 @@ func WithdrawalRewardBalanceCallback(k Keeper, ctx sdk.Context, args []byte, que

// If there's a rebate portion, fund the community pool with that amount
if rebateAmount.GT(sdkmath.ZeroInt()) {
rebateToken := sdk.NewCoin(tradeRouteCallback.RewardDenom, rebateAmount)
rebateToken := sdk.NewCoin(tradeRoute.RewardDenomOnHostZone, rebateAmount)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are actually the same thing if you look at how it sets RewardDenom before the call

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.

4 participants