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.

Sign up for GitHub

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

Jump to bottom

[JSON-RPC] Implement include_events in get_transactions/get_account_transactions #2874

Closed
wants to merge 1 commit into from
Closed

[JSON-RPC] Implement include_events in get_transactions/get_account_transactions #2874

wants to merge 1 commit into from

Conversation

thefallentree
Copy link
Contributor

Also in this PR

  1. adding UnknownEvent fallback
  2. make fetch_result report actual data returned , to make it easier for debugging.

@thefallentree thefallentree requested review from christinacelee and phoenix-o and removed request for dahliamalkhi March 9, 2020 23:29
@thefallentree thefallentree changed the title Implement include_events in get_transactions/get_account_transactions [JSON-RPC] Implement include_events in get_transactions/get_account_transactions Mar 10, 2020
phoenix-o
phoenix-o reviewed Mar 10, 2020
View reviewed changes
json-rpc/src/views.rs Outdated Show resolved Hide resolved
@christinacelee
Copy link

nice! can we just add tests checking for events in TransactionView?

@thefallentree
Copy link
Contributor Author

nice! can we just add tests checking for events in TransactionView?

done

@phoenix-o phoenix-o self-requested a review March 11, 2020 00:25
@bors-libra
Copy link
Contributor

☔ The latest upstream changes (presumably 8b06342) made this pull request unmergeable. Please resolve the merge conflicts.

christinacelee
christinacelee reviewed Mar 11, 2020
View reviewed changes
json-rpc/src/views.rs Show resolved Hide resolved
json-rpc/src/views.rs Show resolved Hide resolved
json-rpc/src/tests/mock_db.rs Show resolved Hide resolved
@thefallentree
Copy link
Contributor Author

rebased, PTAL

@phoenix-o
Copy link
Contributor

@bors-libra: r+

@bors-libra
Copy link
Contributor

📌 Commit 3689d80 has been approved by phoenix-antigravity

@bors-libra
Copy link
Contributor

⌛ Testing commit 3689d80 with merge 8610e97...

@github-actions
Copy link

Cluster Test Result

all up : 1031 TPS, 2820.0 ms latency, no expired txns

Repro cmd:

  ./scripts/cti --k8s --tag land_8610e970 --run bench --k8s-fullnodes-per-validator=0 --k8s-num-validators=100

@bors-libra
Copy link
Contributor

☀️ Test successful - checks-actions_land_blocking_test, checks-circle_commit_workflow
Approved by: phoenix-antigravity
Pushing 8610e97 to master...

DavidLDill pushed a commit to DavidLDill/libra that referenced this pull request Mar 13, 2020
@thefallentree @DavidLDill
Implement include_events in get_transactions/get_account_transactions
c046e2c 
Closes: diem#2874
Approved by: phoenix-antigravity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phoenix-o phoenix-o Awaiting requested review from phoenix-o

@christinacelee christinacelee Awaiting requested review from christinacelee

Assignees
No one assigned
Labels
cla-signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thefallentree @christinacelee @bors-libra @phoenix-o