-
Notifications
You must be signed in to change notification settings - Fork 38
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
WIP Add Geth RPC to the gateway #1845
Conversation
- align some methods with the geth rpc types
# Conflicts: # tools/walletextension/accountmanager/account_manager.go # tools/walletextension/subscriptions/subscriptions.go # tools/walletextension/wallet_extension.go
# Conflicts: # tools/faucet/faucet/faucet.go # tools/walletextension/accountmanager/account_manager.go # tools/walletextension/subscriptions/subscriptions.go # tools/walletextension/wallet_extension.go
# Conflicts: # tools/walletextension/frontend/src/components/head-seo.tsx
…tudor/OG_geth_RPC # Conflicts: # tools/walletextension/common/constants.go # tools/walletextension/wallet_extension.go
# Conflicts: # integration/obscurogateway/tengateway_test.go # tools/walletextension/httpapi/routes.go # tools/walletextension/test/apis.go # tools/walletextension/wallet_extension.go
# Conflicts: # go/common/viewingkey/viewing_key.go # go/common/viewingkey/viewing_key_messages.go # go/common/viewingkey/viewing_key_signature.go # go/enclave/vkhandler/vk_handler.go # tools/walletextension/common/constants.go
return nil | ||
} | ||
|
||
func decodeAddress(s string) (common.Address, error) { |
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.
This feels iffy - if the hex decode fails the error is set, but we return both attempted bytes conversion plus the error.
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.
it's copy paste from geth. I needed to change that Unumarshall
type MessageGenerator interface { | ||
generateMessage(encryptionToken string, chainID int64, version int) ([]byte, error) | ||
generateMessage(encryptionToken []byte, chainID int64, version int) ([]byte, error) |
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.
This needs comments
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.
good point. One for @zkokelj
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.
Gateway is not serving static files for the website.
If you go in frontend and generate static files npm run build
and then compile and run the gateway you are not able to see the webpage.
@@ -1,4 +1,4 @@ | |||
package faucet | |||
package obscurogateway |
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.
obscurogateway -> tengateway for the whole package
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 just aligned it with the folder name. Let's change this as part of a smaller pr
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.
LGTM if @zkokelj happy with the static stuff
Why this change is needed
Replace the "proxy" approach of the Gateway with the type-safe RPC implementation of geth.
This helps structure the business logic and clarify responsibilities.
What changes were made as part of this PR
[]byte
everywhere. Previously, we were converting between string/byte in various ways.account manager
concept and rely on the database to fetch data in real-time.PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks
https://github.com/ten-protocol/ten-test/actions/runs/8450782099/job/23147694242