-
Notifications
You must be signed in to change notification settings - Fork 130
[PAN-2846] Added eea_getPrivacyPrecompileAddress #1650
[PAN-2846] Added eea_getPrivacyPrecompileAddress #1650
Conversation
{ | ||
"jsonrpc": "2.0", | ||
"id": 1, | ||
"result": 126 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be the full ethereum hex encoded address i.e. "0x00000.....7E"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I don't think.
We had discussed this in a call and decided to keep it int
when we pass it as input, such that it's not confusing to the user. And so the output should be consistent with it and return as int
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you specify it as an integer int the command line - but when you want to use it, it is an ethereum address.
If we return the integer, the caller needs to convert the response into an ethereum address to use it. At the very least we should have instructions on how to interpret the output of this call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you say when you want to use it, it is an ethereum address, when and how would you be using it?
I think if using it is going to require converting the response and doc on the fact that conversion is needed, it probably makes more sense to have it return the address rather than the int.
We did discuss it in the call and the reasoning for keeping it as an int was it's a precompile and all the other precompiles are indentified by ints (I think).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So from a user story point of view I would say "I want a new precompile and it will be the precompile at position x
" where x
is a number.
Then once that precompile is put in position x
the story becomes "I want to use my new precompile at position x
so I will send a transaction to y
" where y
is the ethereum address representing x
.
That is in the general case of precompiles.
The reason we brought in this call is so that dApps can learn where our privacy precompile is so they can filter for private transactions by checking the to
address of every public transaction. If a public transaction is privacy marker transaction, the to
address will be the ethereum address of the privacy precompile (i.e. the y
from the user story above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK - I think we really do need to update eea_getPrivacyPrecompileAddress to return the address in hex then. I was wondering what the 7E address I was seeing in transaction objects was - I can see now it's 126 in hex.
@@ -3738,6 +3738,36 @@ None | |||
The `EEA` API methods are not enabled by default for JSON-RPC. Use the [`--rpc-http-api`](Pantheon-CLI-Syntax.md#rpc-http-api) | |||
or [`--rpc-ws-api`](Pantheon-CLI-Syntax.md#rpc-ws-api) options to enable the `EEA` API methods. | |||
|
|||
### eea_getPrivacyPrecompileAddress | |||
|
|||
Returns the address to which the [privacy pre-compiled contract](../Privacy/Private-Transaction-Processing.md) is mapped. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
precompiled or pre-compiled?
Also, maybe "Returns the address of the..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor suggestions
Added eea_getPrivacyPrecompileAddress