-
Notifications
You must be signed in to change notification settings - Fork 208
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
Transaction failure reason not visible when using fixed gas amount #1466
Comments
Example of how this can be acheived:
"jsonrpc": "2.0",
"id": 1,
"result": {
"gas": 21476,
"failed": true,
"returnValue": "08c379a000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000003706f700000000000000000000000000000000000000000000000000000000000",
"structLogs": [...] |
There does seem to be a well worked function for this in fact: |
For ref, the simple contract I'm using is: pragma solidity ^0.8.0;
contract reverter {
uint public storedData;
error Pop(string message, uint256 value);
constructor() {}
function pop1() pure public {
revert("pop");
}
function pop2() pure public {
revert Pop("bang", 12345);
}
} |
Yes - that's the problem. We need to work on FFTM to pass through the error structure from the original TX down into the I suggest at the same time, we promote |
Ok - FFTM does not actually store the error ABI details, they are held in FireFly core and only passed on prepare. They are a big glob of JSON that is associated with the interface, not the transaction, so storing them in FFTM is a big performance overhead. So a more practical approach is to store the result byes from the receipt in FFTM (as for a non-archive node they won't be available indefinitely), and provide an API to decode them that is passed the error ABI. FireFly core could call this API automatically. |
I'm seeing a slightly different behavior with a // SPDX-License-Identifier: Apache-2.0
pragma solidity ^0.8.10;
// Declares a new contract
contract SimpleStorage {
// Storage. Persists in between transactions
uint256 _x;
// Allows the unsigned integer stored to be changed
function set(uint256 x) public {
_x = x;
emit Changed(msg.sender, _x);
}
// Allows the unsigned integer stored to be changed
function setIf(uint256 x, bool actuallyDoIt) public {
require(actuallyDoIt, "Not going to do it");
_x = x;
emit Changed(msg.sender, _x);
}
// Returns the currently stored unsigned integer
function get() public view returns (uint256 x) {
return (_x);
}
event Changed(address indexed from, uint256 x);
} If I set
So this is a little different than what @peterbroadhurst was seeing with |
@matthew1001 @peterbroadhurst I think this one is done |
I believe so, with the exception of custom error ABI parsing as described in some of the comments. |
FireFly normally reports custom error messages all the way back to the API response which is really helpful for developers. Typically, if a transaction will fail, it will also fail the gas estimation step which happens synchronously with submitting the transaction, so the custom error message is reported immediately.
If I submit a transaction, by invoking a custom contract API and adding a fixed gas amount, the behavior is different.
The request is accepted with a
202
and internally gas estimation is skipped (I think). This results in a failed transaction, but my custom error message does not show up in the Operation Output or Details so it is really hard to know why the transaction failed, especially if I have complex logic that could reject the transaction for one of several reasons.The text was updated successfully, but these errors were encountered: