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: direct allocation strategy & direct allocated event handler #33

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

0xnigir1
Copy link
Collaborator

@0xnigir1 0xnigir1 commented Nov 14, 2024

🤖 Linear

Closes GIT-159

Description

  • DirectAllocationStrategy handler
  • DirectAllocated event handler for DirectAllocationStrategy

Checklist before requesting a review

  • I have conducted a self-review of my code.
  • I have conducted a QA.
  • If it is a core feature, I have included comprehensive tests.

Copy link

linear bot commented Nov 14, 2024

@0xnigir1 0xnigir1 requested review from 0xkenj1, jahabeebs and 0xyaco and removed request for jahabeebs November 14, 2024 15:45
this.dependencies,
).handle();
default:
throw new UnsupportedEventException("Strategy", event.eventName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we improve this error by providing strategyName as optional or maybe create a new custom error for strategies. wdyt ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i buy the first option 🫡

Comment on lines +45 to +52
const round = await roundRepository.getRoundByStrategyAddressOrThrow(
this.chainId,
strategyAddress,
);
const project = await projectRepository.getProjectByIdOrThrow(
this.chainId,
this.event.params.profileId,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

sweet

@0xnigir1 0xnigir1 requested a review from 0xkenj1 November 14, 2024 16:11
Copy link
Collaborator

@0xkenj1 0xkenj1 left a comment

Choose a reason for hiding this comment

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

God

Copy link
Collaborator

@jahabeebs jahabeebs left a comment

Choose a reason for hiding this comment

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

🎉

import { BaseStrategyHandler } from "../index.js";
import { DirectAllocatedHandler } from "./handlers/index.js";

const STRATEGY_NAME = "allov2.DirectAllocationStrategy";
Copy link
Collaborator

Choose a reason for hiding this comment

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

as we add more of these we should extract these into one place

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah probably, one more to go (for now)

@0xnigir1 0xnigir1 merged commit 3dcffdb into dev Nov 14, 2024
6 checks passed
@0xnigir1 0xnigir1 deleted the feat/direct-allocation-strategy branch November 14, 2024 22:28
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.

3 participants