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

Confusing names in the codebase #1409

Closed
garious opened this issue Oct 1, 2018 · 10 comments
Closed

Confusing names in the codebase #1409

garious opened this issue Oct 1, 2018 · 10 comments
Assignees

Comments

@garious
Copy link
Contributor

garious commented Oct 1, 2018

Crdt is one of many Conflict-free Data Types. It should named something that reflects its data, not properties of how that data is composed. NetworkInfo? ClusterInfo?

Account is a strange term for the thing held by the Bank, owned by a Pubkey, and stores the owners tokens and arbitrary data. It could use a word that describes rented computer memory. But if not in the naming paradigm of banks, then Bank will need to change too - like Heap. Sticking with the banking metaphor, I'd prefer "safe deposit box", but that's too long, "safe" is ambiguous and would be confused with Rust's "unsafe". How about Lockbox?

Program is too generic a term for the thing that processes instructions directly and interactively. Interpreter seems like a perfect fit. That also leaves space for calling a sequence of instructions a Program or Script.

Process is probably a fine term for an instance of a program or interpreter, but now would be fine time to consider alternatives.

We use Instruction in two places, within a Transaction we use it to mean an instance of userdata (Vec<u8>), a program_id that tells the network what program knows how to interpret that userdata, and list of pubkey indices (Vec<u8>) where each pubkey represents the owner of some Account. The second place is within the Budget DSL. Here we use it to mean something the Budget interpreter (aka program) knows how to execute. The second place seems like the more natural usage and that the first can be renamed to Bytecode, with fields bytes (instead of userdata), account_indexes (or lockbox_indexes), and program_id (or interpreter_id).

What else?

@garious garious added this to the v0.10 Pillbox milestone Oct 1, 2018
@garious garious self-assigned this Oct 1, 2018
@aeyakovenko
Copy link
Member

@garious i think Account is still appropriate. we can't stray to far from mainstream

@garious
Copy link
Contributor Author

garious commented Oct 14, 2018

Yeah, Account won't be changing. Lockbox was my favorite alternative, but still not a perfect fit, so not worth the change.

@garious
Copy link
Contributor Author

garious commented Oct 14, 2018

No more sweeping name changes planned.

@garious garious closed this as completed Oct 14, 2018
@garious garious reopened this Dec 5, 2018
@garious
Copy link
Contributor Author

garious commented Dec 5, 2018

Proposed changes:

  • Look closely at things with a Program suffix. If it interprets instructions, rename the suffix to Interpreter. If it represents the state that's stored in an account's userdata (and not executable), rename the suffix to State.
  • Rename every "program" directory and "*-program" crate to "interpreter".
  • Rename every variation of program ID to interpreter ID
  • Rename Transaction::instructions to Transaction::script
  • Rename Instruction::userdata to Instruction::arguments
  • Rename Account::userdata to Account::data

@garious garious modified the milestones: v0.10 Pillbox, v0.11 Tabletops Dec 5, 2018
@aeyakovenko
Copy link
Member

@garious a transaction script calls multiple interpreters?

@garious
Copy link
Contributor Author

garious commented Dec 6, 2018

Correct, a script can piece together instructions that are interpreted using different interpreters

@garious
Copy link
Contributor Author

garious commented Dec 7, 2018

Proposed changes:

Leave program alone. Only these:

  • Rename Transaction::instructions to Transaction::script
  • Rename Instruction::userdata to Instruction::argdata
  • Rename Account::userdata to Account::data

Also, rename ReplicateStage to ReplyStage to free up term "replicate" for exclusive use in replicator work.

@garious
Copy link
Contributor Author

garious commented Dec 9, 2018

Punting on "instructions -> script" for now - ROI too low. Down to just:

  • Rename Instruction::userdata to Instruction::argdata
  • Rename Account::userdata to Account::data

@aeyakovenko
Copy link
Member

@garious

Punting on "instructions -> script" for now - ROI too low. Down to just:

  • Rename Instruction::userdata to Instruction::argdata
  • Rename Account::userdata to Account::data

That is pretty good!

@garious
Copy link
Contributor Author

garious commented Feb 14, 2019

Closing in favor of #2761

@garious garious closed this as completed Feb 14, 2019
vkomenda pushed a commit to vkomenda/solana that referenced this issue Aug 29, 2021
jeffwashington pushed a commit to jeffwashington/solana that referenced this issue May 17, 2024
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

No branches or pull requests

2 participants