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: refactor handletransfer to use handlerwithloader and not load entities on handler #21

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

vibern0
Copy link

@vibern0 vibern0 commented Jan 3, 2025

No description provided.

@vibern0 vibern0 marked this pull request as ready for review January 6, 2025 09:39
@teawaterwire
Copy link

is this one still relevant? you mentioned that it didn't change performances?
i started looking

async ({ event, context }) =>
await handleTransfer({
PersonalCRC.Transfer.handlerWithLoader({
loader: async ({ event, context }) => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting that we didn't have this code elsewhere before?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which code? The Token and AvatarBalance were loaded in the handleTransfer method.

avatar_id: event.params.to,
token_id: tokenId,
},
from: allAvatarBalances[i * 2 + 1] ?? {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this sorcery? 😂 why i*2 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because i needs AvatarBalance "from" user and "to" user. Given that they are all in one single array, then it jumps each two entities
0-1
2-3
...

@vibern0
Copy link
Author

vibern0 commented Jan 8, 2025

is this one still relevant? you mentioned that it didn't change performances? i started looking

maybe i should ask envio team. I'm surprised with the results

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants