Refactor the crypto services #2170
Conversation
tudor-malene
changed the title
There was a problem hiding this comment.
LGTM! Big milestone getting rid of that hardcoded key. Couple v minor comments.
Worth getting Stefan/other eyes on it too, the extending entropy stuff seems sensible to me but I don't know if there are gotchas.
Nice that we get away with not having to restart Gateway on testnet redeploy still I think, because the encrypted clients are created on demand 💪
| @@ -25,11 +25,6 @@ import ( | |||
| gethlog "github.com/ethereum/go-ethereum/log" | |||
| ) | |||
|
|
|||
| const ( | |||
| // todo: this is a convenience for testnet testing and will eventually be retrieved from the L1 | |||
| enclavePublicKeyHex = "034d3b7e63a8bcd532ee3d1d6ecad9d67fca7821981a044551f0f0cbec74d0bc5e" | |||
There was a problem hiding this comment.
This key string still appears a few times in the codebase which can probably be cleaned up? In particular can probably delete the stuff about 'EnclavePublicKey' in transaction_injector.go
There was a problem hiding this comment.
good spot. Got rid of them. They weren't used anywhere. Probably a vestige
go/rpc/client.go
Outdated
| @@ -17,6 +17,7 @@ const ( | |||
|
|
|||
| Health = "ten_health" | |||
| Config = "ten_config" | |||
| RPCKey = "ten_rPCKey" | |||
There was a problem hiding this comment.
Not sure if it's case sensitive anyway but this capitalisation seems ugly haha, maybe copilot did that.
There was a problem hiding this comment.
I called the method RPCKey, and the convention is prefix_{lowerCaseFirstChar}{otherChars}
There was a problem hiding this comment.
Fair, I'd probably rename it to RpcKey() to fix that if it doesn't break our linter with a comment to explain it keeps the rpc method name nice lol. I guess that method's not super public facing but a little bit.
Why this change is needed
In preparation for MainNet, we must use randomness generated inside the enclave.
What changes were made as part of this PR
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks