Skip to content

feat: initial AssetSink - #1

Merged
marktoda merged 2 commits into
mainfrom
setup
Jul 15, 2025
Merged

feat: initial AssetSink#1
marktoda merged 2 commits into
mainfrom
setup

Conversation

@marktoda

Copy link
Copy Markdown
Contributor
  • implement basic version of AssetSink and tests
  • Consideration: should we have a batch release function? I opted for no since the releaser can just call it multiple times if needed
  • Consideration: should we have a separate releaseERC20 and releaseNative? I opted to just use currencylibrary for simplicity
  • configuration: currently just using an immutable releaser address but in the future we will likely depend on an immutable config contract instead

- implement basic version of AssetSink and tests
- Consideration: should we have a batch release function? I opted for no
  since the releaser can just call it multiple times if needed
- Consideration: should we have a separate releaseERC20 and
  releaseNative? I opted to just use currencylibrary for simplicity
- configuration: currently just using an immutable releaser address but
  in the future we will likely depend on an immutable config contract
  instead
@marktoda

marktoda commented Jul 11, 2025

Copy link
Copy Markdown
Contributor Author

note I deleted bunch of existing stuff that is unlikely to be used directly, including the old asset sink which had burn builtin

Comment thread src/AssetSink.sol
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.29;

import {Currency, CurrencyLibrary} from "v4-core/types/Currency.sol";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

super nit - it does seem strange to have v4 be a dependency, probably can at some point port these to a generic repo?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yeah was wondering about that too, but assume we will need other v4 stuff (ie IProtocolFeeController etc etc) so thought it was ok to have for now

Comment thread src/AssetSink.sol
/// @param asset The asset to release
/// @param recipient The address to receive the assets
/// @dev Only callable by the releaser address
function release(Currency asset, address recipient) external onlyReleaser {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thoughts on adding a batch functionality? so the releaser could do multiple tokens in one call

@saucepoint saucepoint Jul 14, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should most certainly have batch functionality if we want the efficiency of frequent calls & value accrual

but can go in another PR

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah

Consideration: should we have a batch release function? I opted for no since the releaser can just call it multiple times if needed

definitely down to add later but figured for now simplicity and starting point its ok to go without

@snreynolds snreynolds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

basically no notes, looks great

@marktoda
marktoda merged commit 024f8e1 into main Jul 15, 2025
5 checks passed
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