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

feat: emit metadata in _afterTokenTransfer hook #686

Merged
merged 6 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/abstracts/SablierV2Lockup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,20 @@ abstract contract SablierV2Lockup is
INTERNAL CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @notice Overrides the internal ERC-721 transfer function to update the NFT SVG on external platforms by emitting
/// an event upon transfer.
/// @dev This event is also emitted when the NFT is minted or burned.
function _afterTokenTransfer(
address,
address,
andreivladbrg marked this conversation as resolved.
Show resolved Hide resolved
uint256 streamId,
uint256
)
internal
override
updateMetadata(streamId)
{ }

/// @notice Overrides the internal ERC-721 transfer function to check that the stream is transferable.
/// @dev There are two cases when the transferable flag is ignored:
/// - If `from` is 0, then the transfer is a mint and is allowed.
Expand Down
7 changes: 5 additions & 2 deletions test/fork/LockupDynamic.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,14 @@ abstract contract LockupDynamic_Fork_Test is Fork_Test {
vars.initialLockupDynamicBalance = vars.balances[0];
vars.initialBrokerBalance = vars.balances[1];

// Expect the relevant event to be emitted.
vars.streamId = lockupDynamic.nextStreamId();
vm.expectEmit({ emitter: address(lockupDynamic) });
vars.range =
LockupDynamic.Range({ start: params.startTime, end: params.segments[params.segments.length - 1].milestone });

// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
emit MetadataUpdate({ _tokenId: vars.streamId });
vm.expectEmit({ emitter: address(lockupDynamic) });
emit CreateLockupDynamicStream({
streamId: vars.streamId,
funder: holder,
Expand Down
8 changes: 6 additions & 2 deletions test/fork/LockupLinear.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
/// - It should bump the next stream id.
/// - It should record the protocol fee.
/// - It should mint the NFT.
/// - It should emit a {CreateLockupDynamicStream} event.
/// - It should emit a {MetadataUpdate} event
/// - It should emit a {CreateLockupLinearStream} event.
/// - It may make a withdrawal.
/// - It may update the withdrawn amounts.
/// - It may emit a {WithdrawFromLockupStream} event.
Expand Down Expand Up @@ -158,8 +159,11 @@ abstract contract LockupLinear_Fork_Test is Fork_Test {
vars.createAmounts.brokerFee = ud(params.totalAmount).mul(params.broker.fee).intoUint128();
vars.createAmounts.deposit = params.totalAmount - vars.createAmounts.protocolFee - vars.createAmounts.brokerFee;

// Expect the relevant event to be emitted.
vars.streamId = lockupLinear.nextStreamId();

// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
emit MetadataUpdate({ _tokenId: vars.streamId });
vm.expectEmit({ emitter: address(lockupLinear) });
emit CreateLockupLinearStream({
streamId: vars.streamId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,9 @@ contract CreateWithDeltas_LockupDynamic_Integration_Concrete_Test is
// Expect the broker fee to be paid to the broker.
expectCallToTransferFrom({ from: funder, to: users.broker, amount: defaults.BROKER_FEE_AMOUNT() });

// Expect the relevant event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
emit MetadataUpdate({ _tokenId: streamId });
vm.expectEmit({ emitter: address(lockupDynamic) });
emit CreateLockupDynamicStream({
streamId: streamId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,6 @@ createWithDeltas.t.sol
├── it should bump the next stream id
├── it should record the protocol fee
├── it should mint the NFT
├── it should emit a {MetadataUpdate} event
├── it should perform the ERC-20 transfers
└── it should emit a {CreateLockupDynamicStream} event
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,9 @@ contract CreateWithMilestones_LockupDynamic_Integration_Concrete_Test is
amount: defaults.BROKER_FEE_AMOUNT()
});

// Expect the relevant event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupDynamic) });
emit MetadataUpdate({ _tokenId: streamId });
vm.expectEmit({ emitter: address(lockupDynamic) });
emit CreateLockupDynamicStream({
streamId: streamId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ createWithMilestones.t.sol
│ ├── it should bump the next stream id
│ ├── it should record the protocol fee
│ ├── it should mint the NFT
│ ├── it should emit a {MetadataUpdate} event
│ ├── it should perform the ERC-20 transfers
│ └── it should emit a {CreateLockupDynamicStream} event
└── when the asset does not miss the ERC-20 return value
├── it should create the stream
├── it should bump the next stream id
├── it should record the protocol fee
├── it should mint the NFT
├── it should emit a {MetadataUpdate} event
├── it should perform the ERC-20 transfers
└── it should emit a {CreateLockupDynamicStream} event
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,9 @@ contract CreateWithDurations_LockupLinear_Integration_Concrete_Test is
// Expect the broker fee to be paid to the broker.
expectCallToTransferFrom({ from: funder, to: users.broker, amount: defaults.BROKER_FEE_AMOUNT() });

// Expect the relevant event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
emit MetadataUpdate({ _tokenId: streamId });
vm.expectEmit({ emitter: address(lockupLinear) });
emit CreateLockupLinearStream({
streamId: streamId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,9 @@ contract CreateWithRange_LockupLinear_Integration_Concrete_Test is
amount: defaults.BROKER_FEE_AMOUNT()
});

// Expect the relevant event to be emitted.
// Expect the relevant events to be emitted.
vm.expectEmit({ emitter: address(lockupLinear) });
emit MetadataUpdate({ _tokenId: streamId });
vm.expectEmit({ emitter: address(lockupLinear) });
emit CreateLockupLinearStream({
streamId: streamId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ createWithRange.t.sol
│ ├── it should bump the next stream id
│ ├── it should record the protocol fee
│ ├── it should mint the NFT
│ ├── it should emit a {MetadataUpdate} event
│ ├── it should perform the ERC-20 transfers
│ └── it should emit a {CreateLockupLinearStream} event
└── when the asset does not miss the ERC-20 return value
├── it should create the stream
├── it should bump the next stream id
├── it should record the protocol fee
├── it should mint the NFT
├── it should emit a {MetadataUpdate} event
├── it should perform the ERC-20 transfers
└── it should emit a {CreateLockupLinearStream} event

10 changes: 10 additions & 0 deletions test/integration/concrete/lockup/burn/burn.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ abstract contract Burn_Integration_Concrete_Test is Integration_Test, Lockup_Int
whenCallerAuthorized
givenNFTExists
{
// Expect the relevant event to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: notTransferableStreamId });
lockup.burn(notTransferableStreamId);
vm.expectRevert("ERC721: invalid token ID");
lockup.getRecipient(notTransferableStreamId);
Expand All @@ -160,6 +163,10 @@ abstract contract Burn_Integration_Concrete_Test is Integration_Test, Lockup_Int
// Make the approved operator the caller in this test.
changePrank({ msgSender: users.operator });

// Expect the relevant event to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: streamId });

// Burn the NFT.
lockup.burn(streamId);

Expand All @@ -177,6 +184,9 @@ abstract contract Burn_Integration_Concrete_Test is Integration_Test, Lockup_Int
givenNFTExists
givenTransferableStream
{
// Expect the relevant event to be emitted.
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: streamId });
lockup.burn(streamId);
vm.expectRevert("ERC721: invalid token ID");
lockup.getRecipient(streamId);
Expand Down
9 changes: 6 additions & 3 deletions test/integration/concrete/lockup/burn/burn.tree
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ burn.t.sol
│ └── it should revert
└── given the NFT exists
├── given the NFT is not transferable
│ └── it should burn the NFT
│ ├── it should burn the NFT
│ └── it should emit a {MetadataUpdate} event
└── given the NFT is transferable
├── when the caller is an approved third party
│ └── it should burn the NFT
│ ├── it should burn the NFT
│ └── it should emit a {MetadataUpdate} event
└── when the caller is the owner of the NFT
└── it should burn the NFT
├── it should burn the NFT
└── it should emit a {MetadataUpdate} event
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ abstract contract WithdrawMaxAndTransfer_Integration_Concrete_Test is
emit WithdrawFromLockupStream({ streamId: defaultStreamId, to: users.recipient, amount: withdrawAmount });
vm.expectEmit({ emitter: address(lockup) });
emit Transfer({ from: users.recipient, to: users.alice, tokenId: defaultStreamId });
vm.expectEmit({ emitter: address(lockup) });
emit MetadataUpdate({ _tokenId: defaultStreamId });

// Make the max withdrawal and transfer the NFT.
lockup.withdrawMaxAndTransfer({ streamId: defaultStreamId, newRecipient: users.alice });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ withdrawMaxAndTransfer.t.sol
├── it should update the withdrawn amount
├── it should transfer the NFT
├── it should emit a {WithdrawFromLockupStream} event
└── it should emit a {Transfer} event
├── it should emit a {Transfer} event
└── it should emit a {MetadataUpdate} event
4 changes: 2 additions & 2 deletions test/utils/Precompiles.sol

Large diffs are not rendered by default.