0xMiden / 0xMiden/rust-sdk

update node to `0.16` - pending followups

未關閉
#2,473 0 則留言 0 個 reaction 已指派 1 人 已被 @juan518munoz 認領 在 GitHub 檢視
maintainer
主要語言
Rust
星號
78
分支
129
平均合併
4 天 14 小時
30 天內合併 PR
52

描述

## After #2431 got merged, some comments were left unaddressed:

> Minor but we could simplify this a bit. A few entries link to the issue instead of the PR. Those are more related to the RPC changes on the node so we might skip them here all together.

On `CHANGELOG.md` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3873381970

> Should we be passing SyncedNote here instead of the attachments separately?

On `crates/rust-client/src/sync/note_observer.rs` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3873599708

> Currently `SyncedNote` would hold an attachments field but also `CommittedNote` has attachments inside. Is this redundancy correct? Why is it needed? Maybe we could flatten `SyncedNote`.

On `crates/rust-client/src/rpc/domain/note.rs` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3873728597 https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3873847590

> Is the first check covered by the second? If so let's remove it

On `crates/rust-client/src/rpc/domain/account.rs` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3873846521

> This method of taking note of the attachment contents is a bit hard to follow. After a quick look, not sure what the easiest way to make this clearer is, but maybe we can refactor things more thoroughly in order to make it easier to follow.

On `crates/rust-client/src/rpc/domain/note.rs` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3874122617

> I think having something like ResolvedNoteContent is beneficial here, as it is Some when it was already fetched and also more directly describes its functionality. I guess it was removed in order to make NoteAttachments not be optional but I think I would look into refactoring this more in that direction

On `crates/rust-client/src/rpc/domain/note.rs` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3874164301

> This is fine but I wonder if we can just ship a hardcoded insecure key to make the setup just a bit simpler

On `scripts/start-test-node.sh` https://github.com/0xMiden/rust-sdk/pull/2431#discussion_r3875136851

貢獻指南

開啟貢獻指南

研究方向

Review the merged PR #2431 and its linked discussion comments. The work involves several files: CHANGELOG.md, crates/rust-client/src/sync/note_observer.rs, crates/rust-client/src/rpc/domain/note.rs, crates/rust-client/src/rpc/domain/account.rs, and scripts/start-test-node.sh. Start by understanding the existing structures like SyncedNote, CommittedNote, and NoteAttachments. The goal is to address each comment, which may involve simplifying logs, refactoring data structures, removing redundant checks, and potentially simplifying a test script. Running existing tests for the affected modules will verify changes.

由索引模型根據 Issue 內容生成。

評估

技術堆疊
rust
領域
backend-api-design, tooling
Issue 類型
重構
難度
3/5
預估耗時
1-2 天
活躍度
活躍
描述清晰度
基本清楚
新手友好度
55/100

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。