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.

Sign up for GitHub

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

Jump to bottom

Implement aggregation hook #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Implement aggregation hook #52

wants to merge 1 commit into from

Conversation

remybar
Copy link

@remybar remybar commented Jun 22, 2024 •
edited
Loading

Implements #51.

@JordyRo1
Copy link
Contributor

Hey, can you comment Issue #51, so I can assign you ? Thanks !

@remybar remybar marked this pull request as ready for review June 24, 2024 18:58
@remybar remybar changed the title setup aggregation hook Implement aggregation hook Jun 24, 2024
JordyRo1
JordyRo1 reviewed Jun 27, 2024
View reviewed changes
contracts/src/contracts/hooks/aggregation_hook.cairo
}

#[constructor]
fn constructor(ref self: ContractState, hooks: Span<ContractAddress>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hooks are not meant to be defined in the constructor but retrieved from the metadata. But this might be a little tricky, so I can handle it if necessary

JordyRo1
JordyRo1 reviewed Jun 27, 2024
View reviewed changes
contracts/src/contracts/hooks/aggregation_hook.cairo
let hook_count = self.hook_count.read();
let mut i = 0_usize;
loop {
if i >= hook_count {
Copy link
Contributor

Choose a reason for hiding this comment

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

To be applied to hooks defined in metadata.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JordyRo1 JordyRo1 JordyRo1 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@remybar @JordyRo1