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

Refactor: Forwarding Path #227

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Conversation

0xGabi
Copy link
Contributor

@0xGabi 0xGabi commented Aug 21, 2020

Closes #218

Builds on #225

Note: This is still a draft PR, needs to work better on the logic of how to describe a ForwardingPath.

}

// Return a description of the forwarding path, to be rendered.
async describe(): Promise<ForwardingPathDescription> {
Copy link
Contributor Author

@0xGabi 0xGabi Aug 21, 2020

Choose a reason for hiding this comment

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

Here I decide to use the apps that form the path but I wonder if we might need to know the whole list of installed apps. If that case we need to construct the ForwardingPath with a reference to the organization so we can fetch information like the list of apps.


constructor(data: ForwardingPathDescriptionData) {
this.apps = data.apps
this.describeSteps = data.describeSteps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still not super sure about the data structure we would like to have for the ForwardingPathDescription. I just decided to use a list of PostProcessDescription objects. Those are returned by Radspec. We may probably want to further think about it.


//////// DESCRIPTIONS /////////

async describeScript(script: string): Promise<ForwardingPathDescription> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to expose the describeScript utility to the organization as we already have all the contextual information we need and user and app connectors can leverage directly.

@@ -0,0 +1,94 @@
import { ethers } from 'ethers'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the main logic we are currently using to both encode and decode an evm script and transform it into a ForwardingPathDescrption. I'm not convinced yet about the current data types and how we constructing the description but I think we are going in the right direction. Maybe it's a big topic and we should address it as a separate PR and further discuss it in: #219

@0xGabi 0xGabi mentioned this pull request Aug 21, 2020
@0xGabi 0xGabi requested a review from bpierre August 21, 2020 14:38
@0xGabi 0xGabi added this to In review in Roadmap to v1.0.0 via automation Aug 21, 2020
@0xGabi 0xGabi moved this from In review to In progress in Roadmap to v1.0.0 Aug 24, 2020
@0xGabi 0xGabi merged commit 562c639 into refactor-intent Aug 31, 2020
Roadmap to v1.0.0 automation moved this from In progress to Done Aug 31, 2020
@0xGabi 0xGabi deleted the refactor-forwarding-path branch August 31, 2020 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

1 participant