Skip to content

Conversation

@mbjorkqvist
Copy link
Contributor

@mbjorkqvist mbjorkqvist commented Nov 27, 2025

As preparation for supporting the parsing of blocks that have no tx.op field in the ICRC-3 block, this PR proposes to manually implement the Deserialize trait for Block. Internally, the deserialization implementation then uses both the btype of the Block, as well as the parsed FlattenedTransaction, to parse the Transaction that is subsequently used e.g., by the index to apply the transaction in the block.

@mbjorkqvist
Copy link
Contributor Author

Follow-up PR: PR 7848.

@mbjorkqvist mbjorkqvist marked this pull request as ready for review November 27, 2025 20:44
@mbjorkqvist mbjorkqvist requested a review from a team as a code owner November 27, 2025 20:44
Comment on lines 90 to 96
TransferBuilder {
builder: self,
from,
to,
amount,
spender: Some(spender),
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TransferBuilder {
builder: self,
from,
to,
amount,
spender: Some(spender),
}
self.transfer(from, to, amount).spender(spender)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this! I removed the transfer_from helper method completely, since as you pointed out, we have the transfer to which you can add spender for accomplishing the same thing.

maciejdfinity
maciejdfinity previously approved these changes Dec 2, 2025
@maciejdfinity maciejdfinity dismissed their stale review December 2, 2025 17:40

Let's discuss a bit more.

Copy link
Contributor

@maciejdfinity maciejdfinity left a comment

Choose a reason for hiding this comment

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

As discussed, LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants