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

Can we remove sender in Vote, since signature in it #33

Open
yangby-cryptape opened this issue Mar 22, 2019 · 3 comments
Open

Can we remove sender in Vote, since signature in it #33

yangby-cryptape opened this issue Mar 22, 2019 · 3 comments

Comments

@yangby-cryptape
Copy link
Contributor

Both sender and signature in Vote:
https://github.com/cryptape/cita-proto/blob/b1409605bd4f7cf9b857d43fcad1b44b435df3a6/consensus.proto#L23-L27

And sender can be recovered from signature.

Ref:

@kaikai1024
Copy link
Contributor

Sender is useless for signature?

@yangby-cryptape
Copy link
Contributor Author

sender in Vote

Sender is useless for signature?

sender can be recovered from signature.

And, we could not believe messages from network, so this sender can not be treated as a cache.
We have to calculate it from signature.

sender in SignedTransaction

The sender in SignedTransaction is redundant, too.
Because we can recover public key from UnverifiedTransaction.signature, then we can calculate address (sender) from public key.
But, this sender can be used as a cache (we trust the micro-services in the same node), so we could keep this sender.

https://github.com/cryptape/cita-proto/blob/b1409605bd4f7cf9b857d43fcad1b44b435df3a6/blockchain.proto#L64-L76

@kaikai1024
Copy link
Contributor

yeah.
I notice some codes like signature.recover(&message.crypt_hash()) and pubkey_to_address(&pubkey) == sender. we check the sender from the signature, i'm not sure why do we need it at the beginning.

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