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

F/aragondao handler #8

Merged
merged 5 commits into from
Oct 26, 2023
Merged

F/aragondao handler #8

merged 5 commits into from
Oct 26, 2023

Conversation

nigeon
Copy link
Contributor

@nigeon nigeon commented Oct 19, 2023

Added AragonDAO's handler for the faucet.
Works with multisig, token and addresslist daos.
Multi-network.

@nigeon nigeon force-pushed the f/aragondao-handler branch 2 times, most recently from 0db5b7d to d05229f Compare October 21, 2023 11:31
@nigeon nigeon marked this pull request as ready for review October 23, 2023 07:54
@nigeon nigeon requested a review from p4u October 23, 2023 07:55
aragondaohandler/handler.go Outdated Show resolved Hide resolved
aragondaohandler/handler.go Outdated Show resolved Hide resolved
aragondaohandler/handler.go Outdated Show resolved Hide resolved
}

// Check if the address is an Aragon DAO address by checking to AragonGraphQL
if newRequest.Network != "" {
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit weird, I'd either force the user to define network or not (and check all configured networks until one is found). Probably the second is easier for the UI side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason behind this is offering both possibilities,

  1. Searching in just one specific network
  2. Searching in any network

In the scaffold, we don't know (yet) the network that should be used (as it's "network-agnostic"). I think this is not the case for the Aragon Plugin.

Anyway, will ask about the logic to @jpaulet

@p4u p4u merged commit 4d1ea7c into main Oct 26, 2023
1 check passed
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