[BOOST-5224] feat(evm): add topup functionality to core by Quazia · Pull Request #367 · boostxyz/boost-protocol

Quazia · 2025-01-15T04:07:49Z

Description

This PR introduces a new top-up flow in BoostCore that routes additional tokens into existing incentives, adding topupIncentiveFromBudget (for budget-based tokens) and topupIncentiveFromSender (for user-supplied tokens). Both flows gather all tokens in BoostCore , compute and record protocol fees, and temporarily approve the incentive contract to pull the remainder via its topup function. We remove approval after the transfers. Two things to watch out for in review are that funds can't be pulled out of core if there was a malicious incentive and that all permissions on the budget are respected. Basically we want to avoid loss of funds.

Assumptions:

If sending directly the user has already approved BoostCore for ERC20 tokens or setApprovalForAll for ERC1155 tokens .
If sending from a budget the user must be permissioned on the budget. Approval isn't needed based on how we handle disbursals.
The bytes data_ argument decodes based on the incentives InitPayload struct an example of which is shown below.
Protocol fees are only reserved in BoostCore, and may be transferred or settled at a later time.

New Function Signatures

function topupIncentiveFromBudget(
    uint256 boostId,
    uint256 incentiveId,
    bytes calldata data_,
    address budget
) external nonReentrant;

function topupIncentiveFromSender(
    uint256 boostId,
    uint256 incentiveId,
    bytes calldata data_
) external nonReentrant;

Example of Relevant Struct for _data payload

struct InitPayload {
        address asset;
        Strategy strategy;
        uint256 reward;
        uint256 limit;
        address manager;
    }

This param (_data) should be the InitPayload for the incentive being topped up.

changeset-bot · 2025-01-15T04:07:53Z

⚠️ No Changeset found

Latest commit: fcd9d88

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

sammccord · 2025-01-15T04:08:52Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 73177a0

sammccord · 2025-01-15T04:08:59Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 73177a0

topocount

@Quazia are there any tests for this one?

sammccord · 2025-01-15T20:25:20Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 93adba2

sammccord · 2025-01-15T20:46:16Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 8d0633b

sammccord · 2025-01-15T20:48:29Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against b5ee7b5

sammccord · 2025-01-15T21:37:34Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 795b315

sammccord · 2025-01-16T16:31:02Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 795b315

topocount · 2025-01-16T16:33:35Z

@Quazia just a heads-up I've closed #363 and redirected this pr to main. It still contains changes for BOOST-5200 but no PR is open for that feature branch

sammccord · 2025-01-17T15:23:30Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against ad8d258

sammccord · 2025-01-17T16:08:11Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 3e33311

sammccord · 2025-01-22T17:27:06Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against 59b095d

sammccord · 2025-01-23T15:57:14Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against a6a1ee4

…unted amount

this was a configuration issue in hardhat where the compiler config was located in the wrong part of the config

Now that we're supporting topups, this bug should be fixed, where we increment the maxFee by the disbursemd fee calculation instead of setting it for each individual deposit. This correctly accumulates the max fee payout for managers.

sammccord · 2025-01-24T15:43:37Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against bf1a8da

sammccord · 2025-01-24T15:57:39Z

	Warnings
⚠️	Are you sure you want to be submitting a change without including a changeset? If you're just changing docs or tests, you probably don't need to. See the publishing section of the README for more info.

Generated by 🚫 dangerJS against fcd9d88

topocount

Looks good to me, a few small questions

topocount · 2025-01-24T17:45:52Z

packages/evm/contracts/incentives/ERC20Incentive.sol

+    /// @dev Uses `msg.sender` as the token source, and uses `asset` to identify which token.
+    ///      Caller must approve this contract to spend at least `amount` prior to calling.
+    /// @param amount The number of tokens to top up
+    function topup(uint256 amount) external virtual override onlyOwnerOrRoles(MANAGER_ROLE) {


bit of a bikeshedding question: should this interface be added to AIncentive?

I specifically avoided that cause it'd change all the component interfaces and we're not calling into it from the helpers but I could add it. Would just require more thought around the component interface change for existing incentive modules.

topocount · 2025-01-24T17:47:07Z

packages/evm/contracts/incentives/ERC20PeggedIncentive.sol

+        if (amount == 0) {
+            revert BoostError.InvalidInitialization();
+        }


why have this check? spam avoidance?

We could probably remove it - the check exists in the SDK and I'm generally in favor of less in-contract checks for 'enough rope' type problems

topocount · 2025-01-24T17:56:12Z

packages/sdk/src/BoostCore.ts

+  ) {
+    const feeBps = await this.protocolFee(params);
+    const topupAmount =
+      (totalAmount * FEE_DENOMINATOR) / (FEE_DENOMINATOR + feeBps);


nice!
it might be worth reconstituting this and validating the diff isn't greater than the available budget

Yeah we could add some sort of additional check here - we'd probably want it for all four though. WDYT is it worth it?

I think its better than relying on a relatively terse revert

topocount · 2025-01-24T17:57:36Z

packages/sdk/src/BoostCore.ts

+   * @param {?WriteParams} [params] Additional transaction overrides
+   * @returns {Promise<{ hash: Hex; result: void }>}
+   */
+  public async topupIncentiveFromBudgetPostFee(


soft nit:

Suggested change

public async topupIncentiveFromBudgetPostFee(

public async topupIncentiveFromBudgetIncludingFee(

topocount · 2025-01-24T17:58:01Z

packages/sdk/src/BoostCore.ts

+   * @param {?WriteParams} [params] Additional transaction overrides
+   * @returns {Promise<{ hash: Hex; result: void }>} The transaction hash and simulation result
+   */
+  public async topupIncentiveFromBudgetPreFee(


Suggested change

public async topupIncentiveFromBudgetPreFee(

public async topupIncentiveFromBudgetWithoutFee(

This sort of implies that there won't be a fee which there very much will be.

Yeah pre-fee probably works fine

Quazia changed the base branch from main to kevin/boost-5204-add-topup-functionality-to-budgets January 15, 2025 04:08

github-actions bot added the EVM label Jan 15, 2025

topocount reviewed Jan 15, 2025

View reviewed changes

topocount force-pushed the arthur/boost-5224-add-topup-functionality-to-core branch from 93adba2 to 8d0633b Compare January 15, 2025 20:45

topocount force-pushed the arthur/boost-5224-add-topup-functionality-to-core branch from 8d0633b to b5ee7b5 Compare January 15, 2025 20:47

topocount mentioned this pull request Jan 16, 2025

[BOOST-5204] Implement Budget topup interface #363

Closed

topocount changed the base branch from kevin/boost-5204-add-topup-functionality-to-budgets to main January 16, 2025 16:29

Quazia force-pushed the arthur/boost-5224-add-topup-functionality-to-core branch from ad8d258 to 3e33311 Compare January 17, 2025 16:07

topocount force-pushed the arthur/boost-5224-add-topup-functionality-to-core branch from 59b095d to a6a1ee4 Compare January 23, 2025 15:56

github-actions bot added the SDK label Jan 24, 2025

Quazia added 7 commits January 24, 2025 10:39

feat(evm): topup functionality on incentives contracts

08985f6

test(evm): add topup tests to incentive testing

38b78f9

feat(evm): add topups from budgets to boost core

b96a0aa

fix(evm): modify topupIncentiveFromBudget to pipe accounting correctly

ea04f4c

feat(evm): add approves into topupIncentiveFromBudget

572ddb0

feat(evm): incentives can be toped up without a budget

3129342

fix(evm): revert changes to _getFeeDisbursal

aafd0be

Quazia and others added 17 commits January 24, 2025 10:42

feat(evm): add topup function to ManagedBudgetWithFees

50695db

feat(sdk): account for fee as an additional amount instead of a disco…

3b8b8e0

…unted amount

wip: add integration tests for budget topups

eba78d8

fix(evm): correct total amount handling and encoing

8a5ddbf

test(evm): enable topup fuzz testing

bc9b693

test(evm): uncomment the assertion in the topup fuzz test

903b370

fix(evm): utilize the same compiler configs as foundry

9e33fad

this was a configuration issue in hardhat where the compiler config was located in the wrong part of the config

feat(evm): implement Topup events in core

761912b

feat(evm): tweak topup accounting in topupIncentiveFromSender

64174f7

feat(sdk): add topup payload helpers to incentives

2fca309

feat(sdk): add topup helpers to core

0777286

fix(sdk): typo in getTopupPayload call

39c5743

fix(evm): tweak topup approval flow for sender topup

7103cea

feat(sdk): throw if topup is zero in SDK

626ee6e

fix(sdk): correct asset retrieval in getTopupPayload

6073558

test(sdk): add running incentive topup test

bf1a8da

Quazia force-pushed the arthur/boost-5224-add-topup-functionality-to-core branch from 1c9d8a2 to bf1a8da Compare January 24, 2025 15:42

fix(evm): _getFeeDisbursal merge regression

fcd9d88

Quazia enabled auto-merge (rebase) January 24, 2025 16:12

topocount approved these changes Jan 24, 2025

View reviewed changes

Quazia merged commit 43c154c into main Jan 24, 2025
5 checks passed

Quazia deleted the arthur/boost-5224-add-topup-functionality-to-core branch January 24, 2025 18:04

	public async topupIncentiveFromBudgetPostFee(
	public async topupIncentiveFromBudgetIncludingFee(

	public async topupIncentiveFromBudgetPreFee(
	public async topupIncentiveFromBudgetWithoutFee(

Conversation

Quazia commented Jan 15, 2025

Description

Uh oh!

changeset-bot bot commented Jan 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

topocount left a comment

Choose a reason for hiding this comment

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

sammccord commented Jan 15, 2025

Uh oh!

sammccord commented Jan 16, 2025

Uh oh!

topocount commented Jan 16, 2025

Uh oh!

sammccord commented Jan 17, 2025

Uh oh!

sammccord commented Jan 17, 2025

Uh oh!

sammccord commented Jan 22, 2025

Uh oh!

sammccord commented Jan 23, 2025

Uh oh!

sammccord commented Jan 24, 2025

Uh oh!

sammccord commented Jan 24, 2025

Uh oh!

topocount left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

topocount Jan 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

changeset-bot bot commented Jan 15, 2025 •

edited

Loading

topocount Jan 24, 2025 •

edited

Loading