feat: implemented topic transaction TCK endpots.#1282
feat: implemented topic transaction TCK endpots.#1282Adityarya11 wants to merge 2 commits intohiero-ledger:mainfrom
Conversation
|
Hey @Adityarya11 👋 thanks for the PR! This comment updates automatically as you push changes -- think of it as your PR's live scoreboard! PR Checks✅ DCO Sign-off -- All commits have valid sign-offs. Nice work! ✅ GPG Signature -- All commits have verified GPG signatures. Locked and loaded! ✅ Merge Conflicts -- No merge conflicts detected. Smooth sailing! ✅ Issue Link -- Linked to #1268, #1269, #1270, #1271 (assigned to you). 🎉 All checks passed! Your PR is ready for review. Great job! |
github-actions
bot
added
the
status: needs review
rwalworth
left a comment
rwalworth
left a comment
There was a problem hiding this comment.
Thanks for the contribution @Adityarya11! You're right that TopicService.cc needs some attention - I've left detailed comments below. There's also one cross-cutting blocker I want to call out upfront:
src/tck/CMakeLists.txt is not updated. topic/TopicService.cc must be added as a source file there, otherwise the TCK server won't build. All four issues list this in their acceptance criteria.
Let me know if you have any questions!
| /** | ||
| * The ID of the topic to submit the message to. | ||
| */ | ||
| std::string mTopicId; | ||
|
|
||
| /** | ||
| * The message content to submit. UTF-8 encoding. Will be automatically chunked if the message exceeds the chunk size. | ||
| */ | ||
| std::string mMessage; |
There was a problem hiding this comment.
issue (blocking): Per the TCK spec, both topicId and message should be std::optional<std::string> (using getOptionalJsonParameter in from_json), consistent with how other endpoints handle logically-required fields - the network returns the appropriate error when they're omitted, rather than the JSON-RPC layer rejecting the call upfront.
You'll need to add guards in TopicService.cc when applying these to the transaction.
rwalworth
added
status: needs revision
Signed-off-by: Aditya Arya <arya050411@gmail.com>
Signed-off-by: Aditya Arya <arya050411@gmail.com>
Adityarya11
force-pushed
the
feat/topic-tck-endpt
branch
from
2b61743 to
4277239
Compare
github-actions
bot
added
status: needs review
rwalworth
left a comment
rwalworth
left a comment
There was a problem hiding this comment.
Thanks for the updates @Adityarya11 - the second push resolved most of the blocking issues from the first round. One more thing to address before this can merge.
| response["customFees"] = nlohmann::json::array(); | ||
| for (const auto& fee : info.mCustomFixedFees) | ||
| { | ||
| nlohmann::json customFeeItem; | ||
| customFeeItem["amount"] = std::to_string(fee.getAmount()); | ||
| if (fee.getDenominatingTokenId().has_value()) | ||
| { | ||
| customFeeItem["denominatingTokenId"] = fee.getDenominatingTokenId().value().toString(); | ||
| } | ||
| if (!(fee.getFeeCollectorAccountId() == AccountId())) | ||
| { | ||
| customFeeItem["feeCollectorAccountId"] = fee.getFeeCollectorAccountId().toString(); | ||
| } | ||
| response["customFees"].push_back(customFeeItem); | ||
| } |
There was a problem hiding this comment.
issue (blocking): The customFees objects in the getTopicInfo response are serialized manually with a flat structure (amount, denominatingTokenId, feeCollectorAccountId), but the TCK spec for custom fee responses follows the nested format used by CustomFeeSerializer.h - e.g., { "fixedFee": { "amount": "...", "denominatingTokenId": "..." }, "feeCollectorAccountId": "...", "feeCollectorsExempt": ... }. The feeCollectorsExempt field is also missing entirely.
Please align with the serializer format, or at minimum verify against the TCK test expectations at test-topic-info-query.ts.
2 participants
Description:
Added Topic transaction TCK endpoint.
Related issue(s):
Fixes #1268
Fixes #1269
Fixes #1270
Fixes #1271
Notes for reviewer:
TopicService.cc needs some revision.
Checklist