-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement GetTransactionStatus API #1724
Conversation
cdf98e1
to
4e2f793
Compare
- Also changed some API placeholder names to snakecase Transaction status code is returned as: - 3 if the transaction receipt shows successfully accepted, - 1 if it is not yet accepted - my guess at the most appropriate error from the docs if there is one error - 255 (internal error) for multiple errors
7507364
to
5d196c7
Compare
pub last_modified: String, | ||
#[serde(rename = "modificationState")] | ||
pub modification_state: u64, | ||
pub status: u64, |
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 might be useful to link to https://github.com/Zilliqa/Zilliqa/blob/master/src/common/TxnStatus.h#L23 somewhere so we know what 1
, 3
, etc. mean.
match errors[0] { | ||
ScillaError::CallFailed => 11, | ||
ScillaError::CreateFailed => 255, | ||
ScillaError::OutOfGas => 0, |
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 this should be 22
? Maybe we should use an enum or at least some const
s for these magic numbers and refer back to the ZQ1 source?
Transaction status code is returned as:
As far as I can see, ZQ2 transactions and transaction receipts don't have the same information in them that the ZQ1 GetTransactionStatus API docs expect, so my approach here was to try and get as close as possible while still matching the docs, and presumably we can then later add a new API which matches ZQ2.
Closes #1551