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

Keystone: forwarder + write target #614

Draft
wants to merge 5 commits into
base: anchor-0.29
Choose a base branch
from
Draft

Conversation

archseer
Copy link
Collaborator

No description provided.

@archseer archseer changed the title Keystone forwarder Keystone: forwarder + write target Mar 11, 2024
Copy link

@patrick-dowell patrick-dowell left a comment

Choose a reason for hiding this comment

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

Left a few comments

}

// TODO: encode inputs into data
data = append(data, inputs["report"].([]byte)...)

Choose a reason for hiding this comment

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

looks like we are missing the evaluateParams step from the EVM side. Any reason this isn't included the same way it was done on EVM? Seems weird to pull the data from "report" vs the more complex eval we are doing on EVM. If that's what the TODO is for, no worries :)


tx, err := sdk.NewTransaction(
[]sdk.Instruction{
sdk.NewInstruction(config.ForwarderProgramID, accounts, data),

Choose a reason for hiding this comment

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

why does solana use multiple accounts here?

}

go func() {
// TODO: cast tx.Error to Err (or Value to Value?)

Choose a reason for hiding this comment

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

In this case, we are just returning any errors and no values right? I'm assuming the solana manager doesn't have any sort of transaction receipt or a blockhash to confirm that the transaction has finalized.

@bolekk
Copy link
Contributor

bolekk commented Mar 26, 2024

What are the next steps here? How can we move this forward?

@archseer
Copy link
Collaborator Author

I was hoping to discuss as a parking lot item today, we can merge the current design and only handle success route for now. Failure handling will require one of the suggested solutions which will need changes outside the write target

@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
11 New issues
53.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarLint SonarLint

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.

None yet

3 participants