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

Integrate RBPF CLI in Solana SDK #16505

Closed
dmakarov opened this issue Apr 13, 2021 · 13 comments
Closed

Integrate RBPF CLI in Solana SDK #16505

dmakarov opened this issue Apr 13, 2021 · 13 comments
Assignees
Milestone

Comments

@dmakarov
Copy link
Contributor

Problem

RBPF CLI is not very accessible to developers

Proposed Solution

Move the CLI source code into solana repository and make it part of the SDK.

@dmakarov
Copy link
Contributor Author

cc: @jackcmay @Lichtso @mvines: following up #16454

@mvines
Copy link
Member

mvines commented Apr 13, 2021

Instead of moving the code, we could alternatively just install the latest published version fo the RBPF CLI into the full Solana cli release, like we do for spl-token tool:

"$cargo" $maybeRustVersion install spl-token-cli --root "$installDir"

@jackcmay
Copy link
Contributor

jackcmay commented Apr 13, 2021 via email

@Lichtso
Copy link
Contributor

Lichtso commented Apr 13, 2021

Instead of moving the code, we could alternatively just install the latest published version fo the RBPF CLI into the full Solana cli release, like we do for spl-token tool.

The problem is not only that it is hard to find in the first place, but also that it is not integrated. It is stand alone and the user has to manually extract program and data and then feed them separately into the CLI tool.

To align the cli tool with what is happening in the solana runtime we might need to finally bring rbpf into the monorepo. We might also want to augment program-test so that it can produce the same outputs as the cli tool or somehow run program-tests within the cli tool.

I hope the RBPF repo can stay separate for now. I would move the CLI bin code into the lib part of the crate and then call it from within the monorepo.

@Lichtso
Copy link
Contributor

Lichtso commented Apr 23, 2021

I moved the most of the CLI bin code into the lib, as planned. So with the next release of RBPF we can implement the CLI in the mono repo as well.

@mvines mvines added this to the The Future! milestone May 10, 2021
@dmakarov dmakarov self-assigned this May 13, 2021
@dmakarov
Copy link
Contributor Author

I'd like to get this issue resolved. I'm not sure -- have we reached an agreement about moving the CLI tool into solana repository? If yes, where should I move it? Does sdk/rbpf-cli/ sound ok?

@Lichtso
Copy link
Contributor

Lichtso commented May 20, 2021

How about placing it at the root of the repo, the solana-cli and ledger-tool are similar and also at the root.

@dmakarov
Copy link
Contributor Author

How about placing it at the root of the repo, the solana-cli and ledger-tool are similar and also at the root.

Sounds good to me. If no objections, I'll make rbp-cli subdirectory at the root of the repository.

@jackcmay
Copy link
Contributor

Sounds fine to me too for now. I would still like to see rbpf moved into the mono-repo, the only reason it exists outside is because it was originally a fork

@dmakarov
Copy link
Contributor Author

I would still like to see rbpf moved into the mono-repo, the only reason it exists outside is because it was originally a fork

I think I understand your reasoning -- moving solana_rbpf into the mono-repo would guarantee the synchronization of the rbpf with the bpf_loader. However, if we ever need to bring upstream changes into solana_rbpf, it would be much simpler to do when solana_rbpf and the upstream repository share the common git history.

@dmakarov
Copy link
Contributor Author

The rbpf-cli is now in Solana monorepo. Can I close this issue? New follow-up issues can be created for more specific problems.

@Lichtso
Copy link
Contributor

Lichtso commented Nov 11, 2021

#20881 exposed more of the parameters for the input accounts (in bold).

Now there is:

  • key: Pubkey,
  • owner: Pubkey,
  • is_signer: bool,
  • is_writable: bool,
  • lamports: u64,
  • data: Vec<u8>,

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any activity in past 7 days after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants