Skip to content

feat(serve): implement new ReadTx spec endpoint#500

Closed
lancevincentsalera wants to merge 11 commits intotxpipe:mainfrom
lancevincentsalera:feat/implement-new-spec-endpoints
Closed

feat(serve): implement new ReadTx spec endpoint#500
lancevincentsalera wants to merge 11 commits intotxpipe:mainfrom
lancevincentsalera:feat/implement-new-spec-endpoints

Conversation

@lancevincentsalera
Copy link
Copy Markdown
Contributor

@lancevincentsalera lancevincentsalera commented May 14, 2025

Hello @scarmuega, @SebastienGllmt. This PR implements the introduced ReadTx endpoint in utxrpc/spec: utxorpc/spec#143. I would very much appreciate your reviews and thoughts for this. Let me know if there's anything I need to change. Thank you!

NOTE: This includes changes from utxorpc/spec#155 since ReadTx is directly affected with the inclusion of block information inside tx watchers

Comment thread src/serve/grpc/query.rs Outdated
Copy link
Copy Markdown
Contributor

@gonzalezzfelipe gonzalezzfelipe left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread src/chain/redb/v1.rs Outdated
pub fn get_tx_with_block_data(
&self,
tx_hash: &[u8],
) -> Result<Option<(BlockBody, Vec<u8>)>, Error> {
Copy link
Copy Markdown
Contributor

@SebastienGllmt SebastienGllmt May 25, 2025

Choose a reason for hiding this comment

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

I think BlockBody used everywhere in this PR is the wrong name. A block is very loosely split into two things:

  1. block header
  2. block body

MultiEraBlock consists of the full block (body, header, anything else). That's also what I think is logical to return from utxorpc as well. However, in this PR, you refer to the fully block as "BlockBody" which is misleading because it makes it sound like you're returning only the "body" field of the block (whereas you're actually returning the full block)

Although I think this terminology overloading stems from beyond this PR, since I also this this ambiguity in types like RawBlock

Copy link
Copy Markdown
Contributor Author

@lancevincentsalera lancevincentsalera May 27, 2025

Choose a reason for hiding this comment

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

Makes sense. Thank you for pointing this out @SebastienGllmt ! Already applied the changes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe just to loop in @scarmuega since I'm not sure if there was a separate rationale for calling things "body"

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.

3 participants