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

refactor: events must emit addresses involved in the operation for better dApp indexing #186

Closed
0xneves opened this issue Jan 26, 2024 · 3 comments · Fixed by #188
Closed
Assignees
Labels
enhancement New feature or request Hot Task Doubled points in the reward table

Comments

@0xneves
Copy link
Contributor

0xneves commented Jan 26, 2024

Refactor Request

Describe the Refactor Request

We need more context in the logs because we won't have a backend managing past history in the v1.0.0.
This means when your wallet is connected to the dApp, we will check for indexed events in your address.
We can fetch the swapId from such events, even the arguments used to call the function - for instance the createSwap bidding and asked assets. This is thanks to function handlers from SubGraph. However we need to know which user is part of which swap.

Describe Preferred Solution

  • Every Swap event needs to be followed by the swap.owner or swap.allowed, or both. The wanted solution is described below in the code scope.
  • Renaming acceptee to allowed.
  • Refactor tests involving such events.
  event SwapCreated(uint256 indexed swapId, address indexed owner, address indexed allowed);
  event SwapAccepted(uint256 indexed swapId, address indexed owner, address indexed allowed);
  event SwapCanceled(uint256 indexed swapId, address indexed owner);

Related Code

Current events are the following...

event SwapCreated(uint256 indexed swapId);
event SwapAccepted(uint256 indexed swapId, address indexed accepted);
event SwapCanceled(uint256 indexed swapId);

Additional Context

This is a hot task, the first PR resolving the issue will receive its bonuses.

@0xneves 0xneves added enhancement New feature or request Hot Task Doubled points in the reward table labels Jan 26, 2024
@0xneves 0xneves added this to Swaplace Jan 26, 2024
@0xneves 0xneves added this to the Swaplace v1.0.0 milestone Jan 26, 2024
@0xneves 0xneves moved this to 🔖 TODO in Swaplace Jan 26, 2024
@0xneves 0xneves moved this from 🔖 TODO to 🛠️ In Progress in Swaplace Jan 26, 2024
@blackbeard002
Copy link
Contributor

Hey @0xneves I'm on it

@blackbeard002
Copy link
Contributor

Hey @0xneves ready for review

blackbeard002 added a commit to blackbeard002/swaplace-contracts that referenced this issue Jan 26, 2024
0xjoaovpsantos added a commit to 0xjoaovpsantos/swaplace-contracts that referenced this issue Jan 26, 2024
@0xjoaovpsantos
Copy link
Contributor

Hey @0xneves, I made a PR too: #188

@0xneves 0xneves moved this from 🛠️ In Progress to 🕵️‍♀️ In Review in Swaplace Feb 2, 2024
0xneves added a commit that referenced this issue Feb 2, 2024
refactor: events must emit addresses involved in the operation for better dApp indexing #186
0xjoaovpsantos added a commit to 0xjoaovpsantos/swaplace-contracts that referenced this issue Feb 2, 2024
@0xneves 0xneves moved this from 🕵️‍♀️ In Review to ✅ Done in Swaplace Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment