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

BAL Hookathon - Swap Bond Hook #89

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

Conversation

anassohail99
Copy link

@anassohail99 anassohail99 commented Oct 4, 2024

Swap Bond Hook

image

Swap bond hook is a swap discount utility based hook that equips the pools on balancer v3 to create native incentive campaigns on after swap callback. Projects launching their pools can provide incentives to people buying their token through an incentive campaign in which the token buyers will get a bond nft that represents their buy value. The buyers can redeem reward tokens from the discount campaign based on the value of the token bought.

This contraption will help project attract volume without relying on the 3rd party incentive products and within the safe boundaries of Balancer v3.

Pool Life Cycle Points

This hook primarily uses onRegister and onAfterSwap callback:

  • onRegister: During the onRegister callback, the hook gets connected with a pool created.
  • onAfterSwap : When a user buys the project token, the hook catches the state of the swap in onAfterSwap callback and writes this state against the bond nft id in the discount campaing contract, Finally it mints the bond nft to the user.

Example Use Cases

Suppose Project X is launching XT token on Balancer v3, the project is relatively new and wants to bring volume to the pool. Instead of doing the airdrops or relying on a 3rd party incentive protocol, they integrate the swap bond hook with XT/ETH pool. They create a a campaign of 5000 XT tokens as a reward on 50% initial dynamically decreasing discount on the max trade of 200 XT tokens with a cooldown period of 3 days on reward claim.

This way, the project can essentially create an incentive opportunity for their users while also bringing volume to the project which can kickstart the tokenomics if done right.

This hook is not only for the new pools but the projects can utilise this layer at any stage of their project to invite the influx of token holders.

Code And Architecture Review

Watch the video

Local Test Demo

Watch the video

Discount Formula

For the campaign discount we have opt for a dynamically decreasing discount formula. The projects set an initial discount rate with a deterministic quantity of rewards. The discount rate decreases as the users start redeaming the rewards. Whoever avails the discount first gets a better % as compared to the next user. The update formula demostrate it below.

function _updateDiscount() private {
        campaignDetails.discountRate =
            (_maxDiscountRate * (campaignDetails.rewardAmount - tokenRewardDistributed)) /
            campaignDetails.rewardAmount;
    }

Calculations of user claimable reward

function _getClaimableRewards(UserSwapData memory userSwapData) private view returns (uint256 claimableReward) {
        uint256 swappedAmount = userSwapData.swappedAmount;

        // Calculate claimable reward based on the swapped amount and discount rate
        if (swappedAmount <= _maxBuy) {
            claimableReward = (swappedAmount * campaignDetails.discountRate) / 100e18;
        } else {
            claimableReward = (_maxBuy * campaignDetails.discountRate) / 100e18;
        }
    }

Intended Project Interaction

For Projects

  • Create a pool on balancer v3.
  • Register SwapDiscountHook with the pool
  • Create a campaign by interacting with DiscountCampaignFactory contract and transfer the reward tokens to the campaign.

For Users

  • Buy Project token on Balancer v3 and get the Bond nft as early as possible to avail the maximum discount.
  • Wait for the cooldown period to end.
  • Visit the campaign interface and redeem the reward against their bond nft.

Challenges

Since Hook based architecture among the Dexes is a fairly new and untested idea as there is no practical example on mainnet. It's uncertain to know from the user experience perspective if it will be adopted widely and how easy it will be for the people to integrate the applications or hooks built on top of the dexes in their daily crypto usage.

While building the swap bond hook, we also had some questions related to how much control should a a project must have on the emission of rewards while making a campaign. It both depends on the tokenomics of that individual project and how much control they would like to have on their emissions.

There is ofcourse a room of improvement as the hook goes into the hands of people and projects.

Running Tests

Run the Foundry tests

yarn test

DevX Feedback

The online hookathon is well organized through Dorahacks, Kudos on that. The only feedback we have for the team is that they need to figure out more ways for attracting the hackers to contribute to the eco-system either through long term incentives or inclusion with the team.

Authors

Copy link

vercel bot commented Oct 4, 2024

@Mubashir-ali-baig is attempting to deploy a commit to the Matt Pereira's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@EndymionJkb EndymionJkb left a comment

Choose a reason for hiding this comment

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

I can see how much effort went into this! Seems like a useful concept, and a creative use of NFTs. Might be a tad too complex (see comments) :)

* @param _discountCampaignFactory The address of the discount campaign factory.
*/
constructor(
CampaignDetails memory _campaignDetails,

Choose a reason for hiding this comment

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

This is a nit, but _variableName is for private state variables. When there's a name conflict and we need to alter the name in the signature, we've typically used variableName_ or variableNameParam

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, will be updated in the updated PR


import "@openzeppelin/contracts/token/ERC20/IERC20.sol";

library TransferHelper {

Choose a reason for hiding this comment

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

Is this library needed? Should be able to use @openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol directly for most of this.

Copy link
Author

Choose a reason for hiding this comment

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

Right not needed, replaced it with safeERC20

modifier campaignNotExpired(address pool) {
(, , uint256 expirationTime, , , , , ) = IDiscountCampaign(discountCampaigns[pool].campaignAddress)
.campaignDetails();
if (expirationTime > block.timestamp) {

Choose a reason for hiding this comment

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

Suggested change
if (expirationTime > block.timestamp) {
if (block.timestamp <= expirationTime) {

This is clearer - it hasn't expired if the current time is <= expirationTime.

Copy link
Author

Choose a reason for hiding this comment

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

updated

* @notice Modifier to ensure that the campaign for a given pool has not expired.
* @param pool The address of the liquidity pool.
*/
modifier campaignNotExpired(address pool) {

Choose a reason for hiding this comment

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

This reverts if it hasn't expired, so this name is kind of backwards. You're using it to "restart" a campaign after the current campaign has expired. Consider something like modifier campaignExpired.

I guess this is the only way to do it, since the hook is immutable once deployed. It's good that you can't change it midstream; that would potentially rug the users.

Copy link
Author

Choose a reason for hiding this comment

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

I didn't understand, that this check reverts the update transaction if the owner tries to update the campaign during an ongoing campaign.

* @param tokenAddress Address of the ERC20 token to recover.
* @param tokenAmount Amount of tokens to recover.
*/
function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyOwner {

Choose a reason for hiding this comment

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

This would let the owner remove all the reward tokens, and partial removal would break the discount logic, since it's outside the balance accounting. I don't think anyone would use this with this function here; people just need to not send tokens where they shouldn't. If you just send tokens to the Vault, they're lost (v2 or v3). Same with the factory. Not an obvious security hole there, as the factory shouldn't get any tokens.

You can block people sending ETH if you want (as we do in the v3 Vault). I suppose you could keep track of all the reward tokens, and allow withdrawing tokens that were not rewards, but even that would be hard to do safely (things could be done out of order), and it's just not necessary.

And it would only be correct even then if the owner sent the tokens mistakenly. If somebody else did, you'd have to research and manually return them, etc.

Copy link
Author

Choose a reason for hiding this comment

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

We can also allow the owner to remove access tokens apart from the current ongoing reward, but we can remove this function from the campaign contract for the community's trust.

uint256 private _maxBuy;

/// @notice Maximum discount rate available in the campaign.
uint256 private _maxDiscountRate;

Choose a reason for hiding this comment

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

Document the units here. If this is a percentage, we'd typically say _maxDiscountPercentage, and define it as an 18-decimal FixedPoint value from 0-1. (Here it seems to be used assuming 0-100.)

Copy link
Author

Choose a reason for hiding this comment

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

I've updated the document and variable name

ISwapDiscountHook private _swapHook;

/// @notice Maximum buyable reward amount during the campaign.
uint256 private _maxBuy;

Choose a reason for hiding this comment

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

This is a cap on the trade amount; i.e., rewards are proportional to the swap size, but only up to a point, after which it's flat. Maybe swapAmountCap or something similar?

Copy link
Author

Choose a reason for hiding this comment

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

yes it is swapAmountCap which is related to the rewardAmount so if the swapAmount exceeds the cap then we'll calculate the reward based on the maxCap.

uint256 newTokenId = _shareTokenId++;
address user = IRouterCommon(params.router).getSender();
_campaign.updateUserDiscountMapping(_campaignID, newTokenId, user, params.amountCalculatedRaw, block.timestamp);
_mint(user, newTokenId);

Choose a reason for hiding this comment

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

Interesting to make the swap hook itself an NFT! Minting a new NFT on every swap seems extravagant, though. It would add a fair bit of gas to the critical path, and would get unwieldy for users doing frequent swaps. You could end up with 32 NFTs, and then have to spend the gas to claim them all individually. Probably untenable on mainnet.

The way this is designed, you are rewarding people based on the order of claiming - not the order of swapping - though you do have a cooldown period, so early swappers can claim first. Based on the documentation, this is intended.

But if people are incentivized to claim as soon as possible after swapping, consider simply sending the token "rebate" immediately, vs. going through all this NFT minting and claiming process. That would reward people for swapping early vs. "claiming" early (by essentially doing both simultaneously), but that seems like a valid approach as well. Then we wouldn't need a cooldown period, and the overhead would be just a bit of accounting logic plus a token transfer, and much less onerous for the user.

As it is, an early swapper would have to keep track of the cooldown period, and any who forgot or were busy/distracted could lose the benefit of swapping early if a later swapper claimed first.

You could even enable the swap delta, and return extra tokens right in the swap, though that would prevent this being used on pools that support unbalanced liquidity.

If you want NFTs involved, maybe the Campaign could be an NFT, and you could mint one per user that says they participated in the campaign.

Copy link
Author

Choose a reason for hiding this comment

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

if (campaignAddress == address(0)) revert PoolCampaignDoesnotExist();
(, , , , , , address poolAddress, ) = IDiscountCampaign(campaignAddress).campaignDetails();
if (pool != poolAddress) {
revert PoolAddressCannotBeChanged();

Choose a reason for hiding this comment

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

How could this happen? Should be designed to be impossible, so that you don't need to check for it. (Probably already is; I don't see any way to change the pool address. All you can do is "replace"/"refresh" a campaign with new details, which changes everything at once.)

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you are right the second check shouldn't be possible.
Fixed it

rewardAmount: params.rewardAmount,
expirationTime: block.timestamp + params.expirationTime,
coolDownPeriod: params.coolDownPeriod,
discountRate: params.discountAmount,

Choose a reason for hiding this comment

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

The amount is the rate? Would make the naming consistent (and a percentage, if that's what it is). For instance, rebatePercentage (as it's really a rebate, not a discount).

Generally in this system:

A "percentage" is explicitly named as such, as is represented as an 18-decimal FixedPoint value, using the corresponding library (1 = 1e18).

A "rate" is also represented as an FP value, but represents a ratio or something like that - not a "percentage" like a swap fee.

An "amount" (either raw = native token decimals, or scaled18 = 18 decimal FP) is a quantity of something, usually tokens.

Choose a reason for hiding this comment

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

Also, all these things would need validation in production. For instance, you'd want to make sure the coolDownPeriod isn't 100 years, percentage values are properly bounded (e.g., <= 100%, or some lower maximum value). Maybe the reward amount is bounded by some minimum/maximum that makes sense, etc.

Copy link
Author

Choose a reason for hiding this comment

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

apart from the name from discount to rebate you are suggesting changing the parameter from discountAmount to discountRate right? Since it's percentage.

Choose a reason for hiding this comment

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

If it's a percentage, say "percentage" explicitly (e.g., discountPercentage or rebatePercentage). Rates are usually just numbers (e.g., 1.2, 1.05, 0.88) that might represent some kind of ratio or exchange rate, but isn't a percentage. If you mean a literal 0-100% percentage, as you seem to here, it's best to use "percentage" for clarity.

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.

3 participants