Edition minting is supposed to refund unused fees to caller but forwards all value to FeeManager

Summary

The different variants of mint in the Edition are supposed to refund the caller with the unused amount of ETH. However, since all ETH is forwarded to the FeeManager, no refunds are issued as the Edition contract is always empty.

Vulnerability Detail

All of the different variants of the mint function in the Edition contract expect to refund the caller about unused ETH via the _refundExcess() internal function. For example, this is the implementation one of the mint() functions:

function mint(
    address to_,
    uint256 tokenId_,
    uint256 amount_,
    address referrer_,
    bytes calldata data_
) external payable override {
    // wake-disable-next-line reentrancy
    FEE_MANAGER.collectMintFee{value: msg.value}(
        this, tokenId_, amount_, msg.sender, referrer_, works[tokenId_].strategy
    );

    _issue(to_, tokenId_, amount_, data_);
    _refundExcess();
}

function _refundExcess() internal {
    if (msg.value > 0 && address(this).balance > 0) {
        msg.sender.safeTransferETH(address(this).balance);
    }
}

The implementation of _refundExcess() correctly transfers ETH back to the caller. However, all ETH is transferred in the call to collectMintFee(), and the FeeManager doesn't issue any refund.

This means that all mint variants of mint (mint(), mintWithComment(), mintBatch()) will transfer all ETH to the FeeManager, breaking refunds as the Edition contract will have null balance, even if there was an actual excess in fees.

Proof of concept

function test_missing_refund() public {
    Edition edition = new Edition();
    FeeManager feeManager = new FeeManager(address(0xdeadbeef), address(0xc0ffee), address(new MockSplitFactory()));
    TitlesGraph graph = new TitlesGraph(address(this), address(this));

    edition.initialize(
        feeManager,
        graph,
        address(this),
        address(this),
        Metadata({label: "Test Edition", uri: "ipfs.io/test-edition", data: new bytes(0)})
    );

    edition.publish(
        address(1), // creator
        10, // maxSupply
        0, // opensAt
        0, // closesAt
        new Node[](0), // attributions
        Strategy({
            asset: address(0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE),
            mintFee: 0.01 ether,
            revshareBps: 2500, // 25%
            royaltyBps: 250 // 2.5%
        }),
        Metadata({label: "Best Work Ever", uri: "ipfs.io/best-work-ever", data: new bytes(0)})
    );

    // Normally done by the TitlesCore, but we're testing in isolation
    feeManager.createRoute(edition, 1, new Target[](0), address(0));

    address minter = makeAddr("minter");

    vm.deal(minter, 1 ether);

    vm.prank(minter);
    edition.mint{value: 1 ether}(minter, 1, 1, address(0), new bytes(0));

    // minter didn't get any refund
    assertEq(minter.balance, 0);

    // eth is held at fee manager
    assertEq(address(feeManager).balance, 1 ether - 0.0106 ether);
}