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

feat: add optional CBOR representation to GET_TRANSACTION #270

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

iccicci
Copy link
Contributor

@iccicci iccicci commented Nov 19, 2024

Partially solves #253

@iccicci iccicci force-pushed the feat/GET_TRANSACTION_cbor branch from dde7d59 to 09b3697 Compare November 19, 2024 14:53
@iccicci iccicci force-pushed the refactor/unify_output_cbor branch from 6b5678c to 0b4ce7e Compare November 25, 2024 10:35
@iccicci iccicci force-pushed the feat/GET_TRANSACTION_cbor branch from 09b3697 to 36edc11 Compare November 25, 2024 10:36
@iccicci iccicci force-pushed the refactor/unify_output_cbor branch from 0b4ce7e to 1a90ca0 Compare November 29, 2024 10:14
@iccicci iccicci force-pushed the feat/GET_TRANSACTION_cbor branch 2 times, most recently from 7eee599 to 36b7266 Compare November 29, 2024 12:06
@iccicci iccicci changed the base branch from refactor/unify_output_cbor to master November 29, 2024 12:06
@iccicci iccicci force-pushed the feat/GET_TRANSACTION_cbor branch 3 times, most recently from 99ba285 to 671636e Compare November 29, 2024 12:54
@iccicci iccicci marked this pull request as ready for review November 29, 2024 13:02
let promises = [];

// run thousands of fetchTransactionData
// (promise limiter concurrency set to 500 by default)
for (let i = 0; i < 5000; i++) {
promises.push(
transactionUtils.fetchTransactionData(`txHash-${i}`).then(data => {
transactionUtils.fetchTransactionData(`txHash-${i}`, Math.random() > 0.3).then(data => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
transactionUtils.fetchTransactionData(`txHash-${i}`, Math.random() > 0.3).then(data => {
transactionUtils.fetchTransactionData(`txHash-${i}`, true).then(data => {

Let's just enable it for all calls. Introducing randomness won't make the test more useful and could just make it flaky. We could add another test if it make sense to test cbor=false param, but I think we are good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

7d3b2b9 should solve this

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 please rebase and squash

@iccicci iccicci force-pushed the feat/GET_TRANSACTION_cbor branch from 7d3b2b9 to 5f3ab98 Compare December 2, 2024 08:28
@slowbackspace slowbackspace merged commit e83b7a4 into master Dec 3, 2024
3 checks passed
@iccicci iccicci deleted the feat/GET_TRANSACTION_cbor branch December 5, 2024 14:32
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.

2 participants