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

Added status field to Receipt. #98

Merged
merged 3 commits into from
Mar 15, 2018

Conversation

akuanti
Copy link
Contributor

@akuanti akuanti commented Feb 16, 2018

Addresses #97. I'm not sure if this is the correct way to deal with the possibility of either status or root.

Copy link
Owner

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

Looks good! One comment regarding a type. Might be also worth to add a serialization/deserialization test if there is none.

@@ -57,6 +57,8 @@ pub struct Receipt {
pub contract_address: Option<H160>,
/// Logs generated within this transaction.
pub logs: Vec<Log>,
/// Status: either 1 (success) or 0 (failure).
pub status: Option<u32>,
Copy link
Owner

Choose a reason for hiding this comment

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

Since the spec says QUANTITY it should be encoded as hex. To achieve that we need to use a type from ethereum_types. I think the smallest there is U64 so we should use this.

Corresponding code from Parity:
https://github.com/paritytech/parity/blob/afae75b57cd70d81e968fc66319431871a79ff3d/rpc/src/v1/types/receipt.rs#L53

@akuanti
Copy link
Contributor Author

akuanti commented Feb 20, 2018

Do you mean U64 or H64? The smallest uint I see in ethereum-types is U128, and as far as I can tell, Parity is using its own internal U64 type. If I use H64, I get an error about the length of the string being only 1.

This actually brings up something I've been wondering, which is, what is the relationship between this project and Parity? Is seems like there is quite a bit of overlap between them, particularly the RPC types, but they aren't in sync. Does it make sense to extract these types into a separate crate, kind of like ethereum-types? That way, changes to the RPC spec propagate to both projects.

@tomusdrw
Copy link
Owner

https://github.com/paritytech/primitives/blob/master/ethereum-types/src/uint.rs#L7 There is U64 in ethereum types, perhaps we didn't release a new version yet.

Parity and Web3 are both written in Rust and both communicate with each other. Sharing the types indeed makes perfect sense, although the licenses are currently not compatible. Extracting ethereum-types like library would solve that.

@akuanti
Copy link
Contributor Author

akuanti commented Feb 23, 2018

Okay, I'll update the type when the new version is released.

Sharing the types indeed makes perfect sense, although the licenses are currently not compatible. Extracting ethereum-types like library would solve that.

Is there anything I can do to help in this effort?

@tomusdrw tomusdrw merged commit 32927c5 into tomusdrw:master Mar 15, 2018
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