-
Notifications
You must be signed in to change notification settings - Fork 80
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
Alchemy Experimental #1646
Alchemy Experimental #1646
Conversation
…chemy into alchemy-experimental
src/actions/arcActions.ts
Outdated
// pass in null to not redeem any GenesisProtocol rewards | ||
const originalErrorHandler = observer[1]; | ||
observer[1] = async (_error: any): Promise<any> => { | ||
observer[1] = originalErrorHandler; | ||
return await proposalObj.execute().subscribe(...observer); | ||
}; | ||
await proposalObj.claimRewards(null).subscribe(...observer); | ||
|
||
await tryRedeemProposal(proposalId, accountAddress, observer); |
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.
Sorry I was not clear, what should be happening here is this:
return tryRedeemProposal(proposalId, accountAddress, observer);
suppressNotifyOnSuccess: true, | ||
showNotification: this.props.showNotification, | ||
}); | ||
this.forceUpdate(); |
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.
For some reason this isn't showing up as a change request. Adding a comment to try to force it.
... Then it is just the header we are concerned with updating? If so then the dependencies in the header component are broken and need to be fixed between it and arc.ts, not by calling forceUpdate
. If there is a problem here there could just as easily be the same problem everywhere.
|
@orenyodfat yep, still in an in-between phase with supporting v20, will message back when everything is working again. |
"http_main": "https://api.thegraph.com/subgraphs/daostack/name/daostack/...", | ||
"ws_main": "wss://api.thegraph.com/subgraphs/daostack/name/daostack/...", | ||
"http_main": "https://api.thegraph.com/subgraphs/daostack/name/daostack/TODO", | ||
"ws_main": "wss://api.thegraph.com/subgraphs/daostack/name/daostack/TODO", |
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 an additional helpful reminder
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.
@dOrgJelli When is this meant to be complete?
@orenyodfat @dkent600 everything is working again with v20. Please review and test when you can 🙏 |
@dOrgJelli see #1751 , #1750 |
"http_main": "https://api.thegraph.com/subgraphs/daostack/name/daostack/...", | ||
"ws_main": "wss://api.thegraph.com/subgraphs/daostack/name/daostack/...", | ||
"http_main": "https://api.thegraph.com/subgraphs/daostack/name/daostack/TODO", | ||
"ws_main": "wss://api.thegraph.com/subgraphs/daostack/name/daostack/TODO", |
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.
@dOrgJelli When is this meant to be complete?
suppressNotifyOnSuccess: true, | ||
showNotification: this.props.showNotification, | ||
}); | ||
this.forceUpdate(); |
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.
@dOrgJelli This needs to be addressed
@@ -268,7 +271,7 @@ class Header extends React.Component<IProps, null> { | |||
</div> | |||
</span> : <span></span> | |||
} | |||
{!currentAccountAddress ? | |||
{ !currentAccountAddress || currentAccountAddress === "" ? |
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.
@dOrgJelli You can revert this line
FYI: here's the checklist we've gone through to make sure we're ready to go here dOrgTech#5