Skip to content

Midas withdrawal queue - #954

Closed
aliberkhsl wants to merge 15 commits into
midas-allocatorfrom
midas-withdrawal-queue
Closed

Midas withdrawal queue#954
aliberkhsl wants to merge 15 commits into
midas-allocatorfrom
midas-withdrawal-queue

Conversation

@aliberkhsl

Copy link
Copy Markdown
Contributor

No description provided.

@netlify

netlify Bot commented Jul 14, 2025

Copy link
Copy Markdown

Deploy Preview for acre-dapp-testnet canceled.

Name Link
🔨 Latest commit bda11af
🔍 Latest deploy log https://app.netlify.com/projects/acre-dapp-testnet/deploys/68a6f0b46d8d1900099008f1

@netlify

netlify Bot commented Jul 14, 2025

Copy link
Copy Markdown

Deploy Preview for acre-dapp canceled.

Name Link
🔨 Latest commit bda11af
🔍 Latest deploy log https://app.netlify.com/projects/acre-dapp/deploys/68a6f0b40a8f90000822e665

@nkuba
nkuba changed the base branch from main to midas-allocator July 22, 2025 21:13
Comment thread solidity/contracts/midas/WithdrawalQueue.sol Outdated
Comment thread solidity/contracts/midas/WithdrawalQueue.sol Outdated
Comment thread solidity/contracts/midas/WithdrawalQueue.sol
@nkuba nkuba mentioned this pull request Aug 12, 2025
aliberkhsl added a commit that referenced this pull request Aug 13, 2025
This PR introduces the acreBTC contract and migration of deposits from
stBTC to acreBTC.

#### acreBTC

The acreBTC contract is a copy of the stBTC contract with removed
features related to
debt.

#### Migration

The stBTC contract is enhanced with a migration feature that allows to
migrate
the deposit of an owner to a new ERC-4626 contract.

Once the migration phase is started, the funds are released from the
MezoAllocator
and routed to the stBTC contract.

The MezoAllocator contract is unauthorized from accessing the assets of
the
stBTC contract.

During the migration, new deposits are not allowed.

The owner of the stBTC contract can migrate the deposits of the
depositors
without their approval. The migration is done by burning the shares that
are
being migrated and depositing the assets to the new contract.

The migration takes into account the shares that were minted as debt,
excluding
them from the assets that are being migrated.

⚠️ Tests will be implemented in a follow-up PR. We need to merge this PR
to unblock further implementation of
#954.
@nkuba
nkuba changed the base branch from midas-allocator to main August 14, 2025 19:44
Comment thread solidity/contracts/acreBTC.sol
fee
);

return shares;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's emit an event that the withdrawal was requested.

_transfer(owner, withdrawalQueue, shares);

// Process redemption through queue
WithdrawalQueue(withdrawalQueue).requestRedeem(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's emit an event that the withdrawal was requested.

fee
);

return assets;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we should return the assets here, as it may be incorrectly interpreted by the caller.

The withdraw and redeem functions are defined by ERC4626 standard as the functions transferring the tokens to the receiver, in our case we're just requesting an async withdrawal.

We could introduce a requestRedeem function that will include the logic of requesting the withdrawal from the queue (no need to implement requestWithdraw). And keep the redeem/withdraw function synchronous and releasing just the unallocated funds.

WDYT?

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 I think that could work as well but current implementation should be fine too, it's about the communication I think so if there is no enough idle asset in vault on the FE it should show that we are queueing the withdrawal request but open to add another function as well

Comment thread solidity/contracts/acreBTC.sol Outdated
Comment thread solidity/contracts/midas/WithdrawalQueue.sol Outdated
Comment thread solidity/contracts/midas/WithdrawalQueue.sol Outdated
Comment thread solidity/contracts/midas/WithdrawalQueue.sol Outdated
Comment on lines +250 to +258
if (
!tbtc.approveAndCall(
tbtcVault,
tbtcAmount - exitFee,
_tbtcRedemptionData
)
) {
revert ApproveAndCallFailed();
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we call the BitcoinRedeemer contract to request bridging?

Our subgraphs and dApp expects the BitcoinRedeemer.RedemptionRequested event to be emitted when bridging is initialized. It would be great to keep it that way.

Example of what I propose:

// WithdrawalQueue contract
   function finalizeRedeemAndBridge(
        uint256 _requestId,
        bytes calldata _tbtcRedemptionData
    ) external onlyMaintainer {
        if (withdrawalRequests[_requestId] > 0)
            revert WithdrawalRequestAlreadyCompleted();

        (address redeemer, , , , , bytes redeemerOutputScript) = abi.decode(
            _tbtcRedemptionData,
            (address, bytes20, bytes32, uint32, uint64, bytes)
        );
        if (
            redeemer != withdrawalRequests[_requestId].redeemer ||
            redeemerOutputScript != withdrawalRequests[_requestId].redeemerOutputScript
        )
            revert InvalidRedemptionData(
                redeemer,
                withdrawalRequests[_requestId].redeemer,
                redeemerOutputScript,
                withdrawalRequests[_requestId].redeemerOutputScript
            );

        // Expect the tbtcAmount stored in the request is already reduced by the exit fee.
        tbtcAmount = withdrawalRequests[_requestId].tbtcAmount;

        emit WithdrawalRequestCompleted(
            _requestId,
            tbtcAmount,
            exitFee
        );

        // Request bridging tBTC to Bitcoin.
        tbtcToken.forceApprove(address(bitcoinRedeemer), tbtcAmount);
        bitcoinRedeemer.unmint(tbtcAmount, tbtcRedemptionData);
    }

// BitcoinRedeemer contract
    function unmint(uint256 tbtcAmount, bytes calldata tbtcRedemptionData) {
        // TBTC Token contract owner resolves to the TBTCVault contract.
        if (tbtcToken.owner() != tbtcVault) revert UnexpectedTbtcTokenOwner();

        // Transfer tBTC tokens from the caller to BitcoinRedeemer.
        tbtcToken.safeTransferFrom(msg.sender, address(this), tbtcAmount);

        // slither-disable-next-line reentrancy-events
        emit RedemptionRequested(owner, shares, tbtcAmount);

        if (
            !tbtcToken.approveAndCall(tbtcVault, tbtcAmount, tbtcRedemptionData)
        ) {
            revert ApproveAndCallFailed();
        }
    }

);
}

function updateTbtcVault(address _tbtcVault) external onlyMaintainer {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This function and tbtcVault storage slot could be removed if we move tBTC Bridge integration logic to the BitcoinRedeemer contract, the way I proposed in another comment.

@nkuba
nkuba changed the base branch from main to midas-allocator August 27, 2025 19:39
@nkuba nkuba mentioned this pull request Sep 4, 2025
3 tasks
@nkuba

nkuba commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator

Closing in favor of #968

@nkuba nkuba closed this Sep 4, 2025
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