Skip to content
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

Zingo sync scan transaction #1331

Open
wants to merge 15 commits into
base: dev
Choose a base branch
from

Conversation

Oscar-Pepper
Copy link
Contributor

ontop of #1328

@Oscar-Pepper Oscar-Pepper marked this pull request as ready for review August 20, 2024 18:23
Copy link
Contributor

@fluidvanadium fluidvanadium left a comment

Choose a reason for hiding this comment

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

I think it needs a couple edits. i like how its moving.

zingo-sync/src/scan.rs Show resolved Hide resolved
)
.unwrap();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

// TODO: scan transparent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed! i have added a TODO to my latest PR

}

fn scan_notes<D, Op, N>(
wallet_notes: &mut Vec<N::WalletNote>,
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to pass a mutable vector here? I see that the input is empty. instead of the extra parameter just initialize an empty vector in the function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this way is the simplest and cleanest because the vec is initialised in the right scope. if the vec was created by the scan_notes fn then we would have to return it out of the if let block. this gets more complicated as there will also be outgoing notes and encoded memos that also need returning from the block

@zancas zancas added the sync label Sep 7, 2024
@zancas
Copy link
Member

zancas commented Oct 18, 2024

I pushed a resolution of merge conflicts with latest dev.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants