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

Change ActorId to be exact size array #178

Closed
wants to merge 1 commit into from
Closed

Change ActorId to be exact size array #178

wants to merge 1 commit into from

Conversation

jeffa5
Copy link
Collaborator

@jeffa5 jeffa5 commented Jun 17, 2021

No description provided.

@jeffa5 jeffa5 added the enhancement New feature or request label Jun 17, 2021
@jeffa5 jeffa5 requested a review from alexjg June 17, 2021 16:11
@jeffa5
Copy link
Collaborator Author

jeffa5 commented Jun 17, 2021

This has a surprising performance improvement but I'm not sure on whether it is specifically conforming to the automerge spec, I know that ActorIds should be hex-like but I can't see a defined length. They do document it as a UUID so it should be this specified length. I'll probably need to update the js side to be stricter here too.

@ept
Copy link
Member

ept commented Jun 17, 2021

I would like to avoid hard-coding the size of actorIds because different applications may want to define them differently. For example, some may want to define them to be elliptic-curve public keys in order to be able to have actors sign their changes, and those keys are typically 32 bytes in size.

@jeffa5
Copy link
Collaborator Author

jeffa5 commented Jun 17, 2021

Hmm, interesting. Do you think there will be some standardisation of accepted sizes e.g. 16, 32 and 64 or will it just remain a hex sequence of bytes?

@ept
Copy link
Member

ept commented Jun 18, 2021

I think it would be safest to keep it as an arbitrary-length byte sequence, since it's difficult to foresee what people might want to do in the future. As a better approach, how about having a lookup table that you use to translate between actor IDs (arbitrary-length byte arrays) and actor numbers (small, fixed-size integers)? The mapping between IDs and numbers would be specific to a particular instance of the backend, so numbers need to be translated into IDs for any communication with the outside world, but internally within the backend a small integer is sufficient to identify a particular actor.

@jeffa5
Copy link
Collaborator Author

jeffa5 commented Jun 18, 2021

Yeah, we do have that translation in the actor map, this just simplified bits around the protocol but the reasoning for keeping it variable makes sense.

@jeffa5 jeffa5 closed this Jun 18, 2021
@pvh pvh deleted the actorid-array branch June 16, 2022 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants