-
Notifications
You must be signed in to change notification settings - Fork 1
Internal transaction detection #7
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
Conversation
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 implementation has some vibes. I've tested it in the GUI and can confirm that the change-server is now detecting internal transactions in real-time using a localhost change-server against the GUI.
src/hub.ts
Outdated
| logger.warn('Scan address failed: ' + String(error)) | ||
| logger.warn( | ||
| 'stack trace: ' + JSON.stringify(String(error.stack)) | ||
| ) |
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.
Since the error.stack is a superset of error.message, you'll get double logs. How about:
logger.warn( 'Scan address failed: ' + stackify(error))
...
function stackify(error: unknown): string {
if (error instanceof Error) return error.stack
return JSON.stringify(error)
}
src/plugins/evmRpc.ts
Outdated
| try { | ||
| if (node.value != null) value = BigInt(node.value) | ||
| } catch {} | ||
| if (value > 0n) { |
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 seems like we could just do node.value !== '0' here? Why do we even care about the value? All we need is some kind of action on the address - the engine can sort out the values.
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.
True, maybe be more noisy is a good idea.
| ) | ||
| ) | ||
|
|
||
| const walk = (node: any): void => { |
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.
Oh, and do we really need all the any's in here?
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.
Because viem doesn't support these RPC methods, the "any" cascades.
|
An alternative to this fix to try these RPC calls to detect internal transactions, we can opt to fix this more properly using etherscan: https://docs.etherscan.io/api-reference/endpoint/txlistinternal-blockrange But this is an interim solution that when an RPC node supports these methods, will work at least from what I've tested. It's not the cleanest because it's using RPC methods that aren't typed by the library. |
63b690b to
d90a4c0
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
none