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

Fixes for event log and event handling #55

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ynohtna92
Copy link

@ynohtna92 ynohtna92 commented May 23, 2021

These commits fix the following issues:

Ethereum.php does not handle eth_getLogs at all

Fixed by added an OR with the handling of eth_getFilterChanges as they return the same JSON-RPC array.

Event.php does not handle dynamic bytes properly

Fixed by correctly applying offsets for dynamic bytes.
Decoded in accordance with https://docs.soliditylang.org/en/latest/abi-spec.html

SmartContract.php does not allow for processLog to disable event calls

Added bool to allow developers to disable event calls if they want to handle $event differently.

@digitaldonkey
Copy link
Owner

Sorry it takes so long. Currently redoing my CI so I hope I can continuer with this soon.

@digitaldonkey
Copy link
Owner

image

I can see there is a eth_getLogs method (which does not necessarily mean that it is working well..).
The approach to fix it is a little stange, as it is one of the generated functions.
Can you provide test data for your case so that I may be able to write a unit test for it?

Can you do a micro-change, so that the new circleci pipeline gets triggered and we get a run with unit tests.

@ynohtna92
Copy link
Author

// Using the Binance Smart Chain
$web3 = new Ethereum('https://bsc-dataseed4.binance.org/');

// ABI - https://bscscan.com/address/0x22663e2bbf6524dbbe1fb14a84eb689ca65ef367#code
$contractMeta = json_decode(file_get_contents(APP . '/config/contract_bsc.json'));
$contracts["Contract"] = new NFT(
    $contractMeta->abi,
    $contractMeta->address,
    $web3
);

$logs = $web3->eth_getLogs(new Filter(new EthBlockParam(7858017), new EthBlockParam(7858342), new EthBytes("0x22663e2bbf6524dbbe1fb14a84eb689ca65ef367"), null));

foreach($logs as $filterChange) {
    // Fails here
    $event = $contracts["Contract"]->processLog($filterChange, false);
    var_dump($event);
}

I found that it was failing on processing the logs after obtaining them because the eth_getLogs was not handled in your original code.

@digitaldonkey
Copy link
Owner

Sorry. Forgot to mention, that you need to pull the master branch to get the new CircleCi config

The current source does not currently process events containing dynamic bytes properly. These changes add a corrected offset when handling dynamic bytes.
Adds a missing action so the eth_getLogs method is handled correctly.

Fixes typo in \Ethereum\Datatype\\ -> \Ethereum\DataType\ so classes are correctly assigned.
Only call the user function when eventHandle is true. Allows for $event to be passed without being used in a call.
@ynohtna92
Copy link
Author

Sorry. Forgot to mention, that you need to pull the master branch to get the new CircleCi config

Rebased.

@digitaldonkey
Copy link
Owner

digitaldonkey commented Jun 16, 2021

Jay. It doesn't break anything :D
I'm so happy to have a pipeline again.
But I will need to understand what the difference between the original approach (screenshot) and your solution ist. It had been a while. Thank you for the test data. This will help.

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