-
Notifications
You must be signed in to change notification settings - Fork 0
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: add horizon accounting extension logic #53
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor comments
@@ -43,6 +43,7 @@ const protocolProviderConfigSchema = z.object({ | |||
epochManager: addressSchema, | |||
eboRequestCreator: addressSchema, | |||
bondEscalationModule: addressSchema, | |||
horizonAccountingExtension: addressSchema, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice!
Mind adding an example address for this new field in apps/agent/config.example.yml
?
} catch (error) { | ||
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to catch the error and just re-throw it? Could we skip the catch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can just skip the catch, I was just re-throwing because we were re-throwing in other methods in protocolProvider but we can just leave it out since eboActor will handle
try { | ||
const approvedModules = | ||
await this.horizonAccountingExtensionContract.read.approvedModules([user]); | ||
return approvedModules.slice(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be the same as [...approvedModules]
right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep refactored to be more concise
} catch (error) { | ||
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same catch-throw comment as before
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
🤖 Linear
Closes GRT-164
Description
Note that with the new error handling strategy we are just throwing the error if the transaction does not revert and we are not doing the .walk logic that we previously did with errors in ProtocolProvider