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

bugfix: initialize possibleToken fields #497

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

Andrew7234
Copy link
Collaborator

@Andrew7234 Andrew7234 commented Aug 7, 2023

The analyzer crashed with

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x64bc9d]

goroutine 127299 [running]:
math/big.(*Int).Add(0x0, 0x1?, 0xc0000ab2f0?)
	/usr/local/go/src/math/big/int.go:116 +0x1d
github.com/oasisprotocol/nexus/analyzer/runtime.extractEvents.func4.3({0xc000644b0c, 0x14, 0x14}, {0xc000644b2c, 0x14, 0x14}, {0xc000644b40?, 0xc000602858?, 0x69fab4?})
	/code/go/analyzer/runtime/extract.go:764 +0x5e5
github.com/oasisprotocol/nexus/analyzer/runtime.VisitEVMEvent(0xc0000a5b30, 0xc000602e08)
	/code/go/analyzer/runtime/visitors.go:215 +0x7b9
github.com/oasisprotocol/nexus/analyzer/runtime.extractEvents.func4(0xc0000a5b30)
	/code/go/analyzer/runtime/extract.go:633 +0x456
github.com/oasisprotocol/nexus/analyzer/runtime.VisitSdkEvent(0x8?, 0xc000603220)
	/code/go/analyzer/runtime/visitors.go:148 +0x531
github.com/oasisprotocol/nexus/analyzer/runtime.VisitSdkEvents({0xc000456e00, 0x2, 0x0?}, 0xc000603218?)
	/code/go/analyzer/runtime/visitors.go:158 +0x68
github.com/oasisprotocol/nexus/analyzer/runtime.extractEvents(0xc00054be48?, 0xc000311900?, {0xc000456e00?, 0x2?, 0x6?})
	/code/go/analyzer/runtime/extract.go:529 +0x135
github.com/oasisprotocol/nexus/analyzer/runtime.ExtractRound({0x0, {0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...}, ...}, ...)
	/code/go/analyzer/runtime/extract.go:472 +0x13e7
github.com/oasisprotocol/nexus/analyzer/runtime.(*processor).ProcessBlock(0xc000526230, {0x18e98c0, 0xc000b0c1e0}, 0xc00048e700?)
	/code/go/analyzer/runtime/runtime.go:116 +0x259
github.com/oasisprotocol/nexus/analyzer/block.(*blockBasedAnalyzer).Start(0xc0005262d0, {0x18e9850, 0xc00030f7c0})
	/code/go/analyzer/block/block.go:240 +0x951
github.com/oasisprotocol/nexus/cmd/analyzer.(*Service).Start.func1({0x18e3318?, 0xc0005262d0?})
	/code/go/cmd/analyzer/analyzer.go:387 +0x71
created by github.com/oasisprotocol/nexus/cmd/analyzer.(*Service).Start

The root cause was that the totalSupplyChange could be nil if a previously processed ERC721Approval or ERC721ApprovalForAll event in the block initialized possibleToken.TotalSupplyChange to nil.

Todo:
Might be better to change possibleToken.TotalSupplyChange to not be a pointer since we default to 0 anyway... but big.Int uses pointer receiver methods.

type EVMPossibleToken struct {
	Mutated           bool
	TotalSupplyChange big.Int
}

Testing... wasn't able to connect to a mainnet sapphire grpc endpoint yet locally (only testnet).

@Andrew7234 Andrew7234 changed the title initialize possibleToken fields bugfix: initialize possibleToken fields Aug 7, 2023
@Andrew7234 Andrew7234 force-pushed the andrew7234/bugfix-total-supply-change branch from 7ae6341 to ee8ca42 Compare August 7, 2023 17:08
Copy link
Contributor

@mitjat mitjat left a comment

Choose a reason for hiding this comment

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

Thank you for the quick fix, Andy! I have a suggestion for making it more future-proof.

analyzer/runtime/extract.go Outdated Show resolved Hide resolved
analyzer/runtime/runtime.go Outdated Show resolved Hide resolved
analyzer/runtime/extract.go Outdated Show resolved Hide resolved
@Andrew7234 Andrew7234 enabled auto-merge August 7, 2023 21:45
@Andrew7234 Andrew7234 merged commit f3354c0 into main Aug 7, 2023
@Andrew7234 Andrew7234 deleted the andrew7234/bugfix-total-supply-change branch August 7, 2023 21:50
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