Skip to content

fix(issue-1299): 修复Add operator== to TokenNftTransfer#1313

Closed
milestone-17 wants to merge 1 commit intohiero-ledger:mainfrom
milestone-17:issue-1299-fix-Add-operator==to-TokenNftTransfer
Closed

fix(issue-1299): 修复Add operator== to TokenNftTransfer#1313
milestone-17 wants to merge 1 commit intohiero-ledger:mainfrom
milestone-17:issue-1299-fix-Add-operator==to-TokenNftTransfer

Conversation

@milestone-17
Copy link
Copy Markdown

@milestone-17 milestone-17 commented Apr 4, 2026

Description:

Related issue(s):

Fixes #1299

Notes for reviewer:
Please check the logic in TokenNftTransfer.cc 'TokenNftTransfer.h'

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: landmark-achievement <3451978561@qq.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 4, 2026

Hey @milestone-17 👋 thanks for the PR!
I'm your friendly PR Helper Bot 🤖 and I'll be riding shotgun on this one, keeping track of your PR's status to help you get it approved and merged.

This comment updates automatically as you push changes -- think of it as your PR's live scoreboard!
Here's the latest:


PR Checks

DCO Sign-off -- All commits have valid sign-offs. Nice work!


GPG Signature -- Heads up! The following commits don't have a verified GPG signature:

  • ff07590 fix(issue-1299): 修复Add operator== to TokenNftTransfer

You'll need to sign your commits with GPG (e.g. git commit -S). See the Signing Guide for a step-by-step walkthrough.


Merge Conflicts -- No merge conflicts detected. Smooth sailing!


Issue Link -- Almost there! You are not assigned to the following linked issues: #1299.

Please ensure you are assigned to all linked issues before opening a PR. You can comment /assign on the issue to grab it!


All checks must pass before this PR can be reviewed. You've got this!

@github-actions github-actions bot added the status: needs revision A pull request that requires changes before merge label Apr 4, 2026
@codacy-production
Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

TIP This summary will be updated as you push new changes. Give us feedback

@rwalworth
Copy link
Copy Markdown
Contributor

Thanks for working on this, @milestone-17! I'm going to close this PR for the same reason as #1312 - you haven't completed the two Good First Issues required before taking on a beginner issue. The Good First Issues requirement exists specifically to get contributors comfortable with our workflow, and the issues showing up across both of your PRs (missing GPG signature, submitting without being assigned) are exactly what that process is designed to help you work through first.

I do want to flag two things in the code itself so you're aware of them when you come back to this:

First, the operator== declaration was added to TokenFreezeTransaction.h instead of TokenNftTransfer.h. The declaration needs to live alongside the class it belongs to.

Second, there's a bug in the implementation in TokenNftTransfer.cc - lns.mSenderAccountId is being compared against itself instead of rhs.mSenderAccountId, so the sender account is never actually validated:

// Current (buggy) — lns compared against lns
return lns.mNftId == rhs.mNftId && lns.mSenderAccountId == lns.mSenderAccountId && ...

// Should be — lns compared against rhs
return lns.mNftId == rhs.mNftId && lns.mSenderAccountId == rhs.mSenderAccountId && ...

Once you've got two good-first-issues merged, come back to #1299, comment /assign, and then reopen this with those fixes applied. Feel free to ask questions in the meantime - happy to help!

@rwalworth rwalworth closed this Apr 4, 2026
@rwalworth rwalworth removed the status: needs revision A pull request that requires changes before merge label Apr 4, 2026
@milestone-17
Copy link
Copy Markdown
Author

/assgin

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.

[Beginner]: Add operator== to TokenNftTransfer

2 participants