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

updates sync service to submit transaction gossip to api #3926

Merged
merged 3 commits into from
May 26, 2023

Conversation

hughy
Copy link
Contributor

@hughy hughy commented May 22, 2023

Summary

moves 'syncBlocks' to a separate method

defines 'syncTransactionGossip' method to invoke the 'event/onTransactionGossip' RPC and submit transactions to the API

updates 'sync' to run both of these methods in coroutines

updates 'webApi' with a 'transactions' method. matches transaction serialization with serialization from 'followChainStream' for transactions submitted to API with blocks.

Testing Plan

manual testing: ran syncer locally and synced transaction to testnet api: 5c1fa0c851957d46585addf64567fb6ab93f980d23d0d698a6ab49cfc2eb7e1b

Documentation

Does this change require any updates to the Iron Fish Docs (ex. the RPC API
Reference
)? If yes, link a
related documentation pull request for the website.

[ ] Yes

Breaking Change

Is this a breaking change? If yes, add notes below on why this is breaking and
what additional work is required, if any.

[ ] Yes

Base automatically changed from feature/ifl-914/tx-gossip-stream to staging May 25, 2023 13:26
moves 'syncBlocks' to a separate method

defines 'syncTransactionGossip' method to invoke the 'event/onTransactionGossip'
RPC and submit transactions to the API

updates 'sync' to run both of these methods in coroutines

updates 'webApi' with a 'transactions' method. matches transaction serialization
with serialization from 'followChainStream' for transactions submitted to API
with blocks.
@hughy hughy force-pushed the feature/ifl-887/sync-tx-gossip branch from f75ce7d to 94c8029 Compare May 25, 2023 20:54
@hughy hughy marked this pull request as ready for review May 25, 2023 21:08
@hughy hughy requested a review from a team as a code owner May 25, 2023 21:08

async function commit(): Promise<void> {
await api.transactions(buffer)
buffer.length = 0
Copy link
Member

Choose a reason for hiding this comment

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

does this actually clear the buffer? Never seen this before. I guess that's what it was in the block code before

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this looks a bit odd 🤔

copied from the existing syncer code, but i'll take a closer look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will clear the array without replacing it: https://stackoverflow.com/a/1232046

@danield9tqh
Copy link
Member

Looks mostly good to me. I think we can test this on the testnet first for integration testing

@hughy hughy merged commit 1a445f0 into staging May 26, 2023
3 checks passed
@hughy hughy deleted the feature/ifl-887/sync-tx-gossip branch May 26, 2023 20:27
jowparks pushed a commit that referenced this pull request Jun 21, 2023
* updates sync service to submit transaction gossip to api

moves 'syncBlocks' to a separate method

defines 'syncTransactionGossip' method to invoke the 'event/onTransactionGossip'
RPC and submit transactions to the API

updates 'sync' to run both of these methods in coroutines

updates 'webApi' with a 'transactions' method. matches transaction serialization
with serialization from 'followChainStream' for transactions submitted to API
with blocks.

* reuses client between sync methods

* adds expiration to api transaction writes
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