-
Notifications
You must be signed in to change notification settings - Fork 8
feat: horizon support (Semiotic branch + fixes) #1119
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: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Joseph Livesey <[email protected]>
Signed-off-by: Tomás Migone <[email protected]>
Signed-off-by: Tomás Migone <[email protected]>
Signed-off-by: Tomás Migone <[email protected]>
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.
Still working through this review. I've gone ahead and removed all the extra logging that should not be in here.
src/reports.rs
Outdated
signature: Vec<u8>, | ||
} | ||
|
||
#[cfg(test)] |
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.
What are these tests doing in this module?
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.
hmm not sure, they probably should be in src/receipts.rs
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've removed them in ce5d5ed, did not look like they were useful.
README.md
Outdated
For an overview of TAP see https://github.com/semiotic-ai/timeline-aggregation-protocol. | ||
|
||
The gateway acts as a TAP sender, where each indexer request is sent with a TAP receipt. The gateway | ||
This is a horizon-ready gateway that generates TAP v2 receipts exclusively. It maintains backward compatibility for processing existing TAP v1 receipts but it won't generate new ones. The gateway acts as a TAP sender, where each indexer request is sent with a TAP v2 receipt. The gateway |
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.
maintains backward compatibility for processing existing TAP v1 receipts but it won't generate new ones
What does it mean to maintain backward compatibility, if we're not generating the TAP v1 receipts? I don't think there is anything else we would be doing with them.
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.
Not sure what Joseph meant with that, my guess is that it refers to the tap-aggregator's ability to still process TAP v1 receipts, but that component is not on this repo so I removed that line 80faf65.
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.
The initial comment on this PR should also be updated.
Signed-off-by: Tomás Migone <[email protected]>
Signed-off-by: Tomás Migone <[email protected]>
Signed-off-by: Tomás Migone <[email protected]>
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.
LGTM. Thanks!
Marking as DO NOT MERGE, since this is not being deployed to mainnet yet. I've started a build for a docker image tagged |
Note: This PR contains changes from Semiotic PR for horizon support with additional fixes. Original PR description below.
Horizon Gateway Upgrade
This horizon gateway provides TAP v2 receipt generation.
Overview
This is a horizon-ready gateway that replaces TAP receipt generation with TAP v2 receipts on the existing edgeandnode/gateway.
Key Changes from Pre-Horizon Gateway
TAP Protocol Support
Receipt Generation
create_receipt(allocation, fee)
for v1 receiptscreate_receipt(allocation, fee, payer, data_service, service_provider)
for v2 receiptsID Types
AllocationId
for receipt generation and payment trackingAllocationId
for receipt generation, supports both types for processingReceipt Structure
v1 Receipts (Pre-Horizon):
allocation_id
: Address of the allocationtimestamp_ns
: Timestamp in nanosecondsnonce
: Random noncevalue
: Fee amount in weiv2 Receipts (Horizon):
collection_id
: Collection identifier (derived fromallocation_id
)payer
: Address of the payerdata_service
: Address of the data serviceservice_provider
: Address of the service providertimestamp_ns
: Timestamp in nanosecondsnonce
: Random noncevalue
: Fee amount in weiMigration from Pre-Horizon
For Operators
tap_graph
to support v2 receiptsFor Indexers