Skip to content
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

feat: add erigon_getLogs with timestamp field to erigon rpcdaemon and fix the issue 4982 #4968

Merged
merged 22 commits into from Aug 11, 2022

Conversation

fenghaojiang
Copy link
Contributor

feat: add erigon_getLogs with timestamp field to erigon rpcdaemon

@fenghaojiang
Copy link
Contributor Author

By the way, I fixed the issue 4982 #4982. Plz have a check.

@fenghaojiang fenghaojiang changed the title feat: add erigon_getLogs with timestamp field to erigon rpcdaemon feat: add erigon_getLogs with timestamp field to erigon rpcdaemon and fix the issue 4982 Aug 10, 2022
if end < begin {
return nil, fmt.Errorf("end (%d) < begin (%d)", end, begin)
}
if end > roaring.MaxUint32 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How it can happen? Is there som uint underflow in logic above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The method in roaring AddRange The function uses 64-bit parameters even though a Bitmap stores 32-bit values because it is allowed and meaningful to use [0,uint64(0x100000000)) as a range.

        if end > roaring.MaxUint32 {
		return nil, fmt.Errorf("end (%d) > MaxUint32", end)
	}
	blockNumbers := roaring.New()
	blockNumbers.AddRange(begin, end+1) // [min,max)
        ...

func (rb *Bitmap) AddRange(rangeStart, rangeEnd uint64) {
	if rangeStart >= rangeEnd {
		return
	}
	if rangeEnd-1 > MaxUint32 {
		panic("rangeEnd-1 > MaxUint32")
	}
        ...
}

The code should check or use a 32-bit integer parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code in roaring has a limit. But variables end and start are type uint64. Maybe use uint32 at the beginning?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think yes. Or convert bitmap to bitmap64 (not easy - need to change db format), or convert rangeEnd to uint32 (if such a big value is not an underflow).

Copy link
Collaborator

Choose a reason for hiding this comment

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

because such a big value - maybe not an error, maybe it just way to say: "give me all blocks > N"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I changed it to the latest block when rangeEnd is too big. Plz check it.

convert rangeEnd to latest when range end is a big value that go out of range of MaxUint32
add begin condition in case of bigger than latest block
@AskAlexSharov
Copy link
Collaborator

make lintci-deps
make lint

@AskAlexSharov AskAlexSharov merged commit 12cf311 into ledgerwatch:devel Aug 11, 2022
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.

None yet

2 participants