-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Get logs #1028
Get logs #1028
Conversation
Could you merge the master in and get the CI green? I will merge it because it contains some code improvements, and we can address any further issues separarely |
ethdb/remote/ethbackend.pb.go
Outdated
// is compatible with the proto package it is being compiled against. | ||
// A compilation error at this line likely means your copy of the | ||
// proto package needs to be updated. | ||
const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto 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.
Is this a downgrade?
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 depends on what generator version installed locally.
But i found solution for this: if put exact version to go.mod and install generator by ‘go install’ from app folder (make devtools - does it now) - then it installs version from go.mod file.
So, if run make devtools in current master and re-gen things, then ProtoPackageIsVersion6 must appear
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.
Hmm... I've cleaned module packages and downloaded them once again, and now it generates "SupportPackageIsVersion7". At the end of the day, I updated grpc package and regenerated all files once again. It seems everything is good now and tests are green
Request:
Response with error:
|
thanks for checking, @akme ! I moved your comment into the issue, because I want to merge this and we can continue fixing in a new PR |
if len(logs) > 0 { | ||
// We have matching logs, check if we need to resolve full logs via the light client | ||
if logs[0].TxHash == (common.Hash{}) { | ||
receipts := rawdb.ReadReceipts(api.dbReader, header.Hash(), header.Number.Uint64(), params.MainnetChainConfig) |
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.
Could you remove the hard-coded config? There is a way to read it from the DB (you can see in the getReceipt
method or similar)
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.
Done
cmd/rpcdaemon/commands/eth_api.go
Outdated
@@ -3,6 +3,7 @@ package commands | |||
import ( | |||
"context" | |||
"fmt" | |||
ethereum "github.com/ledgerwatch/turbo-geth" |
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.
Wow, I've never seen that one before, what does it import?
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.
Found it. It is interfaces.go
@JekaMas I think you forgot to include the function |
@akme nice catch, could you try again in the latest commit? |
end = crit.ToBlock.Int64() | ||
} | ||
|
||
if begin > end { |
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.
Are you sure we need to error out when being == end or just return empty result?
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 would check pre-existing behaviour
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.
As far as I see go-ethereum just ignores this case.
eth/filters/api.go
Outdated
@@ -502,11 +501,11 @@ func (args *FilterCriteria) UnmarshalJSON(data []byte) error { | |||
args.BlockHash = raw.BlockHash | |||
} else { | |||
if raw.FromBlock != nil { | |||
args.FromBlock = big.NewInt(raw.FromBlock.Int64()) | |||
args.FromBlock = rpc.NewRPCBlockNumber(int(raw.FromBlock.Int64())) |
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.
Aren't raw.FromBlock
and args.FromBlock
of the same type, can you just do assignment?
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.
Not really, golang doesn't allow &
operation for type conversion but we have to convert int64 to int or rpc.BlockNumber.
interfaces.go
Outdated
@@ -134,8 +135,8 @@ type ContractCaller interface { | |||
// FilterQuery contains options for contract log filtering. | |||
type FilterQuery struct { | |||
BlockHash *common.Hash // used by eth_getLogs, return logs only from block with this hash | |||
FromBlock *big.Int // beginning of the queried range, nil means genesis block | |||
ToBlock *big.Int // end of the range, nil means latest block | |||
FromBlock *rpc.BlockNumber // beginning of the queried range, nil means genesis block |
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.
If you are defining FromBlock
and ToBlock
to be of these types, perhaps you don't need UnmarshalJSON
function for FilterQuery
type anymore? My understanding is that it only existed to convert *rpc.BlockNumber
to *big.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.
I still need UnmarshalJSON for Account
that can be either a single value or a slice
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've reverted this type change
rpc/types.go
Outdated
@@ -69,6 +69,11 @@ type jsonWriter interface { | |||
|
|||
type BlockNumber int64 | |||
|
|||
func NewRPCBlockNumber(n int) *BlockNumber { |
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.
Do you still need this function?
go.sum
Outdated
@@ -432,8 +432,8 @@ google.golang.org/grpc v1.21.0/go.mod h1:oYelfM1adQP15Ek0mdvEgi9Df8B9CZIaU1084ij | |||
google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyacEbxg= | |||
google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY= | |||
google.golang.org/grpc v1.27.0/go.mod h1:qbnxyOmOxrQa7FizSgH+ReBfzJrCY1pSN7KXBS8abTk= | |||
google.golang.org/grpc v1.30.1 h1:oJTcovwKSu7V3TaBKd0/AXOuJVHjTdGTutbMHIOgVEQ= | |||
google.golang.org/grpc v1.30.1/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak= | |||
google.golang.org/grpc v1.33.0-dev h1:c0EY3sGPLj50wEdGQDpiS3zvk/zdduzrAkJTfa9ocjY= |
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.
Is dev
an unstable version?
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.
@AskAlexSharov I think only you have the right package and libs versions, I've tried to install versions from master and run devtools
many times and always get packageVersion7
.
Could you checkout to this branch and regenerate files from proto files and then push results to the branch?
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.
- "Is dev an unstable version?" - 1.31 their current release, 1.32-dev their next release, 1.33-dev their next-next release.
- Go code generates by google.golang.org/grpc/cmd/protoc-gen-go-grpc - it means we looking for version reproducibility of this particular tool.
- When I run
make devtools
in master - it added linegoogle.golang.org/grpc/cmd/protoc-gen-go-grpc
to go.mod file (I thought that I added this live with exact version already). Reason why it's not there:go mod tidy
removes it because our source code doesn't use it - it's true - we don't use this package from source code, but we use it to generate go code. - So, probably we can trick
go mod tidy
- add dummy package where I will use all our binary dependencies. But nope, we can do it only if such dep has non-main package: cmd/go: accept main packages as dependencies in go.mod files golang/go#32504 - But I found how to trick it anyway: Reproducible versions of binary dependencies #1037
- I pointed
google.golang.org/grpc/cmd/protoc-gen-go-grpc
to commit ofv1.30.1
tag
worked well |
Hi, guys. |
closes #937
@akme It'd be great if you check the getLogs API.