-
Notifications
You must be signed in to change notification settings - Fork 80
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
fix receipt and pending receipt schema #195
Conversation
42caa12
to
b1da75c
Compare
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.
Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ArielElp and @ShahakShama)
a discussion (no related file):
Can you explain in high level what are the changes?
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.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @ShahakShama and @yair-starkware)
api/starknet_api_openrpc.json
line 383 at r2 (raw file):
"result": { "name": "result", "schema": {
Add a description around here that specifies that the former should be returned for txs not in the pending block and the latter is only for txs in the pending block
api/starknet_api_openrpc.json
line 1444 at r2 (raw file):
] }, "PENDING_BLOCK_BODY_WITH_RECEIPTS": {
If the entire purpose of PENDING_TXN_RECEIPT after this PR is to hardcode the finality status, then I suggest removing all the PENDING_TXN_RECEIPTS related objects, and remove PENDING_BLOCK_BODY_WITH_RECEIPTS. I think everyone is much cleaner with your TXN_RECEIPT and TXN_RECEIPT_WITH_BLOCK_INFO, where get txn receipts returns one of them and getBlockWithReceipts returns the former (regardless of pending/non-pending block)
b1da75c
to
ab4160c
Compare
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.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @ArielElp and @yair-starkware)
a discussion (no related file):
Previously, yair-starkware (Yair) wrote…
Can you explain in high level what are the changes?
Mainly fixing bugs in the schema of receipts and making it more defined
api/starknet_api_openrpc.json
line 383 at r2 (raw file):
Previously, ArielElp wrote…
Add a description around here that specifies that the former should be returned for txs not in the pending block and the latter is only for txs in the pending block
Done.
api/starknet_api_openrpc.json
line 1444 at r2 (raw file):
Previously, ArielElp wrote…
If the entire purpose of PENDING_TXN_RECEIPT after this PR is to hardcode the finality status, then I suggest removing all the PENDING_TXN_RECEIPTS related objects, and remove PENDING_BLOCK_BODY_WITH_RECEIPTS. I think everyone is much cleaner with your TXN_RECEIPT and TXN_RECEIPT_WITH_BLOCK_INFO, where get txn receipts returns one of them and getBlockWithReceipts returns the former (regardless of pending/non-pending block)
Done.
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.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yair-starkware)
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.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @ShahakShama)
This change is