Skip to content

Commit

Permalink
Do not emit events from AbstractTBTCDepositor (#786)
Browse files Browse the repository at this point in the history
In all cases considered so far, the child contract emits its own events
in the functions calling `_initializeDeposit` and `_finalizeDeposit`.
The events emitted by child contracts have the advantage of emitting
decoded extra data in the parameters. We could emit events from the
parent contract as well but they cost gas and we are emitting the same
data in child contract events. Also, we sometimes need to be creative in
child contracts and come up with other names than `DepositInitialized`
and `DepositFinalized` even though that is exactly what happens :)

Also, lesson learned today: if there is a contract inheriting from
`AbstractTBTCDepositor` and declaring an event with the same name - say
`DepositFinalized` - when testing the contract whether the event was
emitted, Hardhat will complain this event does not exist in the
contract.
  • Loading branch information
lukasz-zimnoch authored Feb 14, 2024
2 parents b9fa855 + 3c7dcb9 commit e3c317e
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 48 deletions.
22 changes: 0 additions & 22 deletions solidity/contracts/integrator/AbstractTBTCDepositor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,6 @@ abstract contract AbstractTBTCDepositor {
// slither-disable-next-line unused-state
uint256[47] private __gap;

event DepositInitialized(uint256 indexed depositKey, uint32 initializedAt);

event DepositFinalized(
uint256 indexed depositKey,
uint256 tbtcAmount,
uint32 finalizedAt
);

/// @notice Initializes the contract. MUST BE CALLED from the child
/// contract initializer.
// slither-disable-next-line dead-code
Expand Down Expand Up @@ -150,12 +142,6 @@ abstract contract AbstractTBTCDepositor {
reveal.fundingOutputIndex
);

emit DepositInitialized(
depositKey,
/* solhint-disable-next-line not-rely-on-time */
uint32(block.timestamp)
);

// The Bridge does not allow to reveal the same deposit twice and
// revealed deposits stay there forever. The transaction will revert
// if the deposit has already been revealed so, there is no need to do
Expand Down Expand Up @@ -211,14 +197,6 @@ abstract contract AbstractTBTCDepositor {

tbtcAmount = _calculateTbtcAmount(deposit.amount, deposit.treasuryFee);

// slither-disable-next-line reentrancy-events
emit DepositFinalized(
depositKey,
tbtcAmount,
/* solhint-disable-next-line not-rely-on-time */
uint32(block.timestamp)
);

extraData = deposit.extraData;
}

Expand Down
26 changes: 0 additions & 26 deletions solidity/test/integrator/AbstractTBTCDepositor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,6 @@ describe("AbstractTBTCDepositor", () => {
.withArgs(fixture.expectedDepositKey)
})

it("should emit the DepositInitialized event", async () => {
await expect(tx)
.to.emit(depositor, "DepositInitialized")
.withArgs(fixture.expectedDepositKey, await lastBlockTime())
})

it("should return proper values", async () => {
await expect(tx)
.to.emit(depositor, "InitializeDepositReturned")
Expand Down Expand Up @@ -233,16 +227,6 @@ describe("AbstractTBTCDepositor", () => {
await restoreSnapshot()
})

it("should emit the DepositFinalized event", async () => {
await expect(tx)
.to.emit(depositor, "DepositFinalized")
.withArgs(
fixture.expectedDepositKey,
expectedTbtcAmount,
await lastBlockTime()
)
})

it("should return proper values", async () => {
await expect(tx)
.to.emit(depositor, "FinalizeDepositReturned")
Expand Down Expand Up @@ -277,16 +261,6 @@ describe("AbstractTBTCDepositor", () => {
await restoreSnapshot()
})

it("should emit the DepositFinalized event", async () => {
await expect(tx)
.to.emit(depositor, "DepositFinalized")
.withArgs(
fixture.expectedDepositKey,
expectedTbtcAmount,
await lastBlockTime()
)
})

it("should return proper values", async () => {
await expect(tx)
.to.emit(depositor, "FinalizeDepositReturned")
Expand Down

0 comments on commit e3c317e

Please sign in to comment.