Change to Event Emitted in Escrow.sol

[arthurgousset]

Change to Event Emitted in Escrow.sol

Hi all 👋

@eelanagaraj and team are working on updates in Escrow.sol to:

1. add support for federated attestations (FederatedAttestations.sol)
2. fix a small existing bug
3. fix an inconsistency in the data emitted by the Withdrawal event 👈 Topic of this thread

Update in emitted Withdrawal event

The Withdrawal event has the following fields:

event Withdrawal(
  bytes32 indexed identifier,
  address indexed to, // <--- to field
  address indexed token,
  uint256 value,
  address paymentId
);

and is emitted when the function withdraw() is called:

// function withdraw() [BEFORE]
    ...
    emit Withdrawal(
      payment.recipientIdentifier, 
      payment.sender, // <--- to field = payment.sender
      payment.token,
      payment.value,
      paymentId
    );
    ...

Notice that the to field is set to the payment.sender instead of the msg.sender who will actually receive the withdrawal.
We would like to update the data emitted in this event to be msg.sender.:

// function withdraw() [AFTER]
    ... 
    emit Withdrawal(
      payment.recipientIdentifier,
      msg.sender, // <--- to field = msg.sender
      payment.token,
      payment.value,
      paymentId
    );
    ...

What does this mean?

This change would be a PATCH upgrade, according to our Core Contracts versioning process, because it is:

• backwards compatible, and
• doesn't change the ABI.

⚠️ But (!), this change would modify the data captured by applications that index the Escrow.sol events emitted on-chain. From the moment the contract is upgraded, the to field would reflect msg.sender instead of payment.sender.

🙌 Call to action👇

Please comment below if you are indexing Escrow.sol events and might be affected by this change.

We would like to share this update ahead of time so (d)apps that use the contract or index its events can prepare accordingly.

Thanks!