Skip to content

fix(#79): compare wallet key and full transaction content - #134

Open
bibonix wants to merge 1 commit into
zold-io:masterfrom
bibonix:79-equal-wallets-full-content
Open

fix(#79): compare wallet key and full transaction content#134
bibonix wants to merge 1 commit into
zold-io:masterfrom
bibonix:79-equal-wallets-full-content

Conversation

@bibonix

@bibonix bibonix commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

The puzzle in Copies.java asked the equalWallets helper to compare wallets by more than just id and ledger size; the RSA key and every transaction field were supposed to participate. Until now any two wallets sharing only id and transaction count were collapsed into a single Copies.Copy, hiding genuinely divergent ledgers.

equalWallets now checks id, key, and ledger size up front, then walks both ledgers in parallel and compares each transaction across id, time, amount, prefix, beneficiary, details, and signature. The field comparison is split between equalNumbers and equalText so no single boolean expression breaches the Qulice complexity limit. The puzzle marker is removed from the source.

Local checks: mvn -B test (68 tests, 0 failures) and mvn -B -Pqulice -DskipTests verify both pass.

Closes #79

equalWallets in Copies only checked id and ledger size, treating
distinct wallets as identical whenever those two numbers agreed. It
now also compares the RSA key and every transaction field-by-field
across id, time, amount, prefix, beneficiary, details, and signature.

@edmoffo edmoffo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implementation covers the puzzle: id, key, ledger size, then per-transaction comparison across the seven Transaction fields, split into helpers to stay inside Qulice's complexity bound. Two concerns worth raising. The key() call introduces a hard dependency on Wallet.File.key(), which still throws UnsupportedOperationException per @todo #54 in Wallet.java near line 280 (see inline comment). Separately, the new equality fan-out (id mismatch, key mismatch, ledger-size mismatch, per-field transaction mismatch) is not exercised by CopiesTest, and the existing @todo #56:30min at CopiesTest.java:17 for the missing scenarios is untouched. CI is green.

final List<Transaction> head = new ListOf<>(first.ledger());
final List<Transaction> tail = new ListOf<>(second.ledger());
boolean equal = first.id() == second.id()
&& first.key().equals(second.key())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

first.key().equals(second.key()) reaches Wallet.File.key(), which still throws UnsupportedOperationException (Wallet.java near line 280, @todo #54). Once two File wallets land here with the same id the short-circuit lets execution reach this expression and it throws instead of letting equalWallets return cleanly. Either gate this branch behind Wallet.File.key actually being implemented (#54 lands first), or have equalWallets treat an unsupported key() as equal-when-ids-match so the comparison degrades gracefully until #54.

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.

Copies.java:85-87: Compare the entire content of two...

2 participants