Skip to content

feat: nitro-rpc proto and golang draft implementation - #2

Merged
mod merged 12 commits into
feat/nitro-rpc-appfrom
feat/nitro-rpc-proto-impl
Mar 18, 2025
Merged

feat: nitro-rpc proto and golang draft implementation#2
mod merged 12 commits into
feat/nitro-rpc-appfrom
feat/nitro-rpc-proto-impl

Conversation

@philanton

Copy link
Copy Markdown

No description provided.

Comment thread nitro_rpc/test/NitroRPCTest.t.sol Outdated
Comment thread nitro_rpc/src/NitroRPC.sol Outdated
Comment thread nitro_rpc/remappings.txt Outdated
Comment thread nitro_rpc/web/server.go Outdated

state := RPCState{
RequestID: req.Req.GetRequestId(),
Timestamp: req.Req.GetTimestamp(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we set a new timestamp here, so that an external party can calculate how much time a response took place to calculate?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I discussed with @mnaichuk timestamp will be later generated by server, so that is TBD


privateKey, err := crypto.GenerateKey()
require.NoError(t, err)
ss := signer.NewLocalSigner(privateKey)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you shorten signer to ss?)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because signer is the name of the package, and ss stands for stateSigner, I could instead use rename for package as signer_pkg

@mod
mod merged commit 21b5560 into feat/nitro-rpc-app Mar 18, 2025
@mod
mod deleted the feat/nitro-rpc-proto-impl branch March 18, 2025 13:09
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

Successfully merging this pull request may close these issues.

3 participants