-
Notifications
You must be signed in to change notification settings - Fork 5
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: add icrc3_get_transactions #41
Conversation
cycles-ledger/tests/tests.rs
Outdated
// Replacing expected blocks with the actual blocks is needed because only actual | ||
// blocks have the timestamp |
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.
This comment is a bit confusing, at least to me.
Effectively, you are extracting the actual block because it has the timestamp set, which you need to compute the correct hash, right?
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.
Yes. The timestamp cannot be computed so I need to fetch it. I could just change the timestamp field of the block instead to make it more clear.
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.
Updating the comment to make it clearer or changing the timestamp field, either way works for me. You choose! :-)
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 done both with c65cad0. I set the timestamp and I changed the description to explain why I do it. Is it better?
cycles-ledger/tests/tests.rs
Outdated
// Replacing expected blocks with the actual blocks is needed because only actual | ||
// blocks have the timestamp |
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.
Same here.
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
The PR is missing a description. Could you add one, please? |
Done! |
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
Co-authored-by: Thomas Locher <[email protected]>
icrc3_get_transactions
endpoint following the ICRC-3 standard