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] N2N TxSubmission #63

Merged
merged 31 commits into from
Apr 13, 2024
Merged

[FEAT] N2N TxSubmission #63

merged 31 commits into from
Apr 13, 2024

Conversation

nemo83
Copy link
Contributor

@nemo83 nemo83 commented Apr 5, 2024

Feat N2N TxSubmission

This is the implementation of the Node-to-Node mini protocol TxSubmission.

Worth to mention that I've replaced the initial HashMap containing ids and bytes of the txs as I wanted to preserve order to addition (as it actually matters).

I've streamed real node mempool via N2C to a local node via N2N TxSubmission and it has been running for a few hours w/o errors or disconnecting.

@nemo83 nemo83 requested a review from satran004 April 5, 2024 14:38
Copy link
Member

@satran004 satran004 left a comment

Choose a reason for hiding this comment

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

Thanks @nemo83 for the PR.

Looks good to me. Few minor comments.

map.put(new ByteString(HexUtil.decodeHexString(id)), new UnsignedInteger(size));
var pair = new Array();
var era = new Array();
era.add(new UnsignedInteger(5));
Copy link
Member

Choose a reason for hiding this comment

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

Is this an era specific value or a constant value (always 5) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oppssieee a daisy. It's an Era specific. Great catch, will look how it's handled elsewhere in the code and update accordingly, but happy to get pointers / suggestions if you have any on how to pass or where to get that info!

Copy link
Member

@satran004 satran004 Apr 9, 2024

Choose a reason for hiding this comment

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

Check Era enum in package com.bloxbean.cardano.yaci.core.protocol.localstate.api . Probably you need this one. This one is used in local state query and local tx submission.

There is another Era enum, but it has different values and is used in block de-serialization if I am not wrong.

helper/build.gradle Outdated Show resolved Hide resolved
@nemo83
Copy link
Contributor Author

nemo83 commented Apr 8, 2024

Updated most comments, before closing I need to:

  • Check how to properly set the Era in the serialization code. Where can I get such info?
  • Test that after replacing Vector w/ ConcurrentLinkedQueue it all still works (as I've changed the code removing transactions)

@satran004
Copy link
Member

Thanks @nemo83 . Please check my era specific comment above.

Is it possible to add an integration test for Tx submission? Something like LocalTxSubmissionClientIT.

@satran004
Copy link
Member

@nemo83 I tried it and I was able to successfully submit and execute my transaction

So, the callback methods in listener are only to notify if the transaction has been successfully pulled by the remote node. right?

@satran004 satran004 merged commit 88bd2e9 into main Apr 13, 2024
1 check passed
@satran004 satran004 deleted the feat/n2n-txsubmisison branch April 13, 2024 04: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.

None yet

2 participants