-
Notifications
You must be signed in to change notification settings - Fork 87
feat: Integrate ntx-builder with validator #1453
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
base: next
Are you sure you want to change the base?
Conversation
crates/ntx-builder/src/rpc.rs
Outdated
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.
Unfortunately this won't work as the RPC rejects network transactions since these are not allowed to be sent publicly (yet).
Options:
- Add the validator directly to ntx -- this seems unreasonable.
- Open up network transactions publicly
- Add authentication somehow
- An additional internal RPC url which the ntx uses which only supports the ntx submit tx method but skips the rejection
I haven't done (3) with gRPC before, so I would automatically reach for auth via http but https://grpc.io/docs/guides/auth/ has some more info.
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.
Can you share some more context on the relationship of auth / network transactions beyond to give some context? (link is enough)
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.
There isn't really any direct context aside from the fact that currently we only allow our own network transaction builder component to submit network transactions. Allowing external submissions would cause races with our builder and we don't want to deal with that at this stage.
This is currently enforced in the RPC component's SubmitProvenTransaction implementation by simply rejecting all transactions that target a network account (except for those which create the account). This means external clients/users can only create a network account and thereafter all transactions for that account cannot go via the public RPC endpoints.
The network tx builder circumvents this by using the block-producer's SubmitProvenTransaction directly. This works but now that we're adding additional logic (validator stuff) to the RPC's gRPC implementation it would be ideal if we only have that in a single place.
Network transactions must also be submitted to the validator, so either we
- duplicate the validator interaction logic within the network transaction builder as well, or
- keep a single coherent impl in the RPC component
- and therefore the RPC component must somehow allow network transactions from the internal network transaction builder without allowing them in from the general public
- one option for this is some sort of authentication to just say "hey this came from our internal network transaction builder, don't reject it"
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.
I don't think option 2 is viable. Network transactions shouldn't come from the outside at this point.
Why is option 1 unreasonable? NTX builder could send the transaction directly to the validator in the same way as the RPC does. It feel like this would take the least amount of work.
My order of preference would probably be:
- Option 3, if it is not difficult to do. We could send the API key in the headers somehow - though, I remember working with headers was not super straight-forward - but maybe now we have this solved.
- Option 1 - just sent transactions from the NTX builder directly to the validator. As I mentioned above, this should be pretty straight-forward to implement.
- Option 4 - though, running two separate RPCs (one for internal use and one for external use) may introduce more complexity than the other two approaches.
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.
Summarizing: go with option 1 - ntx-builder talks directly to validator
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.
Moved forward with Option 1 (ready for review again)
…x-builder-validator
Context
We want network transactions to go through the same validator flow as all other transactions. Currently the ntx-builder submits proven transactions to the block producer directly which bypasses the validator flow.
Closes #1309.
Changes
ValidatorClientfor ntx-builder.BlockProducerClient::submit_proven_transactionfor ntx-builder.ValidatorClient::submit_proven_transaction.