From d8212038a90830b2228363b067cf807911ef8461 Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:08:55 -0400 Subject: [PATCH 1/5] fix(walletapi): tighten receiver sender_idx bounds check to prevent panic The receiver decode paths used sender_idx <= RingSize against a 0-indexed Publickeylist of length RingSize, so a crafted byte equal to RingSize indexed one slot past the slice and panicked the receiving wallet during history processing. Tighten to < in both the CBOR and CBOR_V2 paths so valid indices are 0..RingSize-1. --- walletapi/daemon_communication.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/walletapi/daemon_communication.go b/walletapi/daemon_communication.go index eaa1db38..2cffea05 100644 --- a/walletapi/daemon_communication.go +++ b/walletapi/daemon_communication.go @@ -1071,7 +1071,7 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err } } - if sender_idx <= uint(tx.Payloads[t].Statement.RingSize) { + if sender_idx < uint(tx.Payloads[t].Statement.RingSize) { // off-by-one fix: valid indices are 0..RingSize-1 addr := rpc.NewAddressFromKeys((*crypto.Point)(tx.Payloads[t].Statement.Publickeylist[sender_idx])) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() @@ -1110,7 +1110,7 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err } } - if sender_idx <= uint(tx.Payloads[t].Statement.RingSize) { + if sender_idx < uint(tx.Payloads[t].Statement.RingSize) { // off-by-one fix: valid indices are 0..RingSize-1 addr := rpc.NewAddressFromKeys((*crypto.Point)(tx.Payloads[t].Statement.Publickeylist[sender_idx])) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() From 1a0e0bac0565f833217bc2c746ff48da45a621ee Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Fri, 12 Jun 2026 16:09:25 -0400 Subject: [PATCH 2/5] feat(walletapi): surface sender attribution as unverified for rings > 2 Add SenderVerified and RingSize to rpc.Entry, populated in both receiver decode paths. SenderVerified is true only at ring size 2, where the receiver override pins attribution to the counterparty; for larger rings the sender chose the unauthenticated attribution byte, so entry.Sender must not be presented as trusted. RingSize is carried so consumers can render attribution confidence per entry. --- rpc/wallet_rpc.go | 8 +++++++- walletapi/daemon_communication.go | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/rpc/wallet_rpc.go b/rpc/wallet_rpc.go index 11f1a480..badcedfc 100644 --- a/rpc/wallet_rpc.go +++ b/rpc/wallet_rpc.go @@ -82,7 +82,13 @@ type Entry struct { Payload_RPC Arguments `json:"payload_rpc,omitempty"` // these fields are only valid based on payload type and if payload could be successfully parsed and will by default be equal to zero values - Sender string `json:"sender"` + Sender string `json:"sender"` + // SenderVerified is true ONLY when attribution is protocol-pinned (ring size 2, + // where the counterparty is necessarily the sender). For ring size > 2 the sender + // chose the unauthenticated attribution byte and entry.Sender MUST NOT be trusted. + SenderVerified bool `json:"sender_verified"` + // RingSize is the ring size of the payload this entry was decoded from (0 if unknown). + RingSize uint64 `json:"ringsize"` DestinationPort uint64 `json:"dstport"` SourcePort uint64 `json:"srcport"` } diff --git a/walletapi/daemon_communication.go b/walletapi/daemon_communication.go index 2cffea05..8677756c 100644 --- a/walletapi/daemon_communication.go +++ b/walletapi/daemon_communication.go @@ -1076,6 +1076,9 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() } + entry.RingSize = uint64(tx.Payloads[t].Statement.RingSize) + // ring size 2 attribution is protocol-pinned (set above); larger rings are sender-chosen and unverified + entry.SenderVerified = uint(tx.Payloads[t].Statement.RingSize) == 2 entry.Payload = append(entry.Payload, tx.Payloads[t].RPCPayload[1:]...) entry.Data = append(entry.Data, tx.Payloads[t].RPCPayload[:]...) @@ -1115,6 +1118,9 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() } + entry.RingSize = uint64(tx.Payloads[t].Statement.RingSize) + // ring size 2 attribution is protocol-pinned (set above); larger rings are sender-chosen and unverified + entry.SenderVerified = uint(tx.Payloads[t].Statement.RingSize) == 2 entry.Payload = append(entry.Payload, payload[1:]...) entry.Data = append(entry.Data, payload...) From 4e9de56445f322cdc9c5b9a8e5e5f71e5482c351 Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Sat, 20 Jun 2026 18:55:51 -0400 Subject: [PATCH 3/5] fix(walletapi): scrub unverified sender attribution from exported entry fields For ring sizes greater than 2 the receiver-decode attribution byte is sender-chosen and unauthenticated, yet entry.Sender was set from it and the raw byte was shipped in the exported entry.Data. Because the ring Publickeylist is public, a consumer could re-derive the claimed sender from entry.Data[0] even when entry.Sender was trusted, letting a sender name any ring member as the apparent sender of a payment they never made. Blank entry.Sender and zero the leading slot byte of the entry.Data copy when !SenderVerified, so an unverified attribution cannot be re-derived from any exported field. The verified (ring==2) path and the self/send decode sites are unchanged. Enforces the SenderVerified flag added in 1a0e0ba, which until now had no consumer. A source-level guard locks the scrub at both receiver decode paths so a regression fails loud. This is a decode-time scrub and does not retroactively clean entries persisted before the change. --- walletapi/attribution_export_guard_test.go | 55 ++++++++++++++++++++++ walletapi/daemon_communication.go | 28 ++++++++++- 2 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 walletapi/attribution_export_guard_test.go diff --git a/walletapi/attribution_export_guard_test.go b/walletapi/attribution_export_guard_test.go new file mode 100644 index 00000000..972bf792 --- /dev/null +++ b/walletapi/attribution_export_guard_test.go @@ -0,0 +1,55 @@ +package walletapi + +import ( + "os" + "regexp" + "testing" +) + +// Test_AttributionExportGuard_UnverifiedSlotByteScrubbed locks the export-boundary +// scrub for an unverified sender attribution (ring > 2). +// +// When a received transfer's attribution is unverified, payload[0] is a sender-chosen, +// unauthenticated slot index. It must NOT survive into any exported Entry field: the raw +// byte re-derives the claimed sender via the public Publickeylist even after entry.Sender +// is blanked, because entry.Data carries the whole payload and Publickeylist is public. +// +// This guard pairs with Test_AttributionGuard_HonestWritesReceiverIndex (the send-side +// guardrail). It is a source-level guard (greps the two receive-as-receiver decode sites) +// so it needs no wallet/daemon harness; the behavioral end-to-end proof lives in the +// simulator harness. Its job is to make a future refactor that drops the scrub fail loud. +func Test_AttributionExportGuard_UnverifiedSlotByteScrubbed(t *testing.T) { + src, err := os.ReadFile("daemon_communication.go") + if err != nil { + t.Fatalf("cannot read daemon_communication.go: %v", err) + } + + // The scrub must blank entry.Sender when the attribution is unverified. + blankSender := regexp.MustCompile(`if\s+!entry\.SenderVerified\s*\{[\s\S]*?entry\.Sender\s*=\s*""`) + if n := len(blankSender.FindAll(src, -1)); n < 2 { + t.Fatalf("EXPORT SCRUB MISSING: expected entry.Sender blanked under `if !entry.SenderVerified` at "+ + "both receive sites (CBOR + CBOR_V2); found %d. An unverified attribution must not export a "+ + "claimed sender string.", n) + } + + // The scrub must zero the leading slot byte in the payload copy that feeds entry.Data, + // so the byte cannot re-derive the sender via Publickeylist. Matches both the CBOR site + // (`tx.Payloads[t].RPCPayload[1:]`) and the CBOR_V2 site (the local `payload[1:]`). + zeroByte := regexp.MustCompile(`append\(\[\]byte\{0x00\},\s*[\w.\[\]]+\[1:\]\.\.\.\)`) + if n := len(zeroByte.FindAll(src, -1)); n < 2 { + t.Fatalf("EXPORT SCRUB MISSING: expected the attribution slot byte zeroed (`append([]byte{0x00}, "+ + "...[1:]...)`) in the exported payload at both receive sites; found %d. Without this, "+ + "entry.Data[0] still re-derives the claimed sender through the public Publickeylist.", n) + } + + // At the two receive-as-receiver sites, entry.Data must be fed from the sanitized + // `exported_payload`, never the raw payload. (The two SELF-side decode sites — where the + // wallet decodes a tx IT sent — legitimately keep the raw payload and are not gated by + // SenderVerified; they are out of scope and intentionally not matched here.) + sanitizedExport := regexp.MustCompile(`entry\.Data\s*=\s*append\(entry\.Data,\s*exported_payload`) + if n := len(sanitizedExport.FindAll(src, -1)); n < 2 { + t.Fatalf("EXPORT SCRUB BYPASSED: expected entry.Data fed from the sanitized `exported_payload` at "+ + "both receive sites; found %d. Feeding the raw payload would export an unverified slot byte "+ + "that re-derives the claimed sender via the public Publickeylist.", n) + } +} diff --git a/walletapi/daemon_communication.go b/walletapi/daemon_communication.go index 8677756c..501c3f2d 100644 --- a/walletapi/daemon_communication.go +++ b/walletapi/daemon_communication.go @@ -1080,8 +1080,20 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err // ring size 2 attribution is protocol-pinned (set above); larger rings are sender-chosen and unverified entry.SenderVerified = uint(tx.Payloads[t].Statement.RingSize) == 2 + // sanitized copy of the decrypted payload for the exported entry fields. + // for an unverified attribution (ring > 2, where payload[0] is sender-chosen + // and unauthenticated) the leading attribution slot byte must NOT be exported: + // it re-derives the claimed sender via the public Publickeylist even after + // entry.Sender is blanked. So we blank entry.Sender AND zero payload[0] in the + // copy that feeds entry.Data. The verified case (ring 2, structural) is untouched. + exported_payload := tx.Payloads[t].RPCPayload + if !entry.SenderVerified { + entry.Sender = "" + exported_payload = append([]byte{0x00}, tx.Payloads[t].RPCPayload[1:]...) + } + entry.Payload = append(entry.Payload, tx.Payloads[t].RPCPayload[1:]...) - entry.Data = append(entry.Data, tx.Payloads[t].RPCPayload[:]...) + entry.Data = append(entry.Data, exported_payload[:]...) args, _ := entry.ProcessPayload() _ = args @@ -1122,8 +1134,20 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err // ring size 2 attribution is protocol-pinned (set above); larger rings are sender-chosen and unverified entry.SenderVerified = uint(tx.Payloads[t].Statement.RingSize) == 2 + // sanitized copy of the decrypted payload for the exported entry fields. + // for an unverified attribution (ring > 2, where payload[0] is sender-chosen + // and unauthenticated) the leading attribution slot byte must NOT be exported: + // it re-derives the claimed sender via the public Publickeylist even after + // entry.Sender is blanked. So we blank entry.Sender AND zero payload[0] in the + // copy that feeds entry.Data. The verified case (ring 2, structural) is untouched. + exported_payload := payload + if !entry.SenderVerified { + entry.Sender = "" + exported_payload = append([]byte{0x00}, payload[1:]...) + } + entry.Payload = append(entry.Payload, payload[1:]...) - entry.Data = append(entry.Data, payload...) + entry.Data = append(entry.Data, exported_payload...) args, _ := entry.ProcessPayload() _ = args From b6e80244575c9b62a0bfd52f992f674e8a17c77d Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Wed, 24 Jun 2026 09:08:33 -0400 Subject: [PATCH 4/5] feat(walletapi): reconcile self-send attribution flag + add behavioral scrub test Addresses the #21 six-perspective review: - Self-send sites set SenderVerified=true + RingSize (both decode arms): a wallet-authored tx has a certain sender at any ring size, so a contract- following consumer no longer distrusts the user's own sends. (review #2) - Add Test_Attribution_Scrub_Behavior: drives the real receiver decode on a sim chain and asserts the scrub effect (ring2 keeps Sender+Data[0]; ring>2 blanks Sender + zeroes Data[0]). Rebuilds until the receiver lands in a non-zero ring slot so the Data[0]==0 assertion has teeth. Mutation-checked. The source-grep guard is kept as a tripwire. (review #3) - Document the (RingSize, SenderVerified, Sender) renderer truth table on the field; drop the redundant exported_payload[:] slice. (review #6, #9) The <=->< bounds guard is sufficient: rpc.NewAddress panics on an undecompressable ring key before the Publickeylist rebuild, so len(Publickeylist)==RingSize holds and the index cannot go out of range. (review #1) --- rpc/wallet_rpc.go | 14 +- walletapi/attribution_scrub_behavior_test.go | 256 +++++++++++++++++++ walletapi/daemon_communication.go | 10 +- 3 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 walletapi/attribution_scrub_behavior_test.go diff --git a/rpc/wallet_rpc.go b/rpc/wallet_rpc.go index badcedfc..af2acdab 100644 --- a/rpc/wallet_rpc.go +++ b/rpc/wallet_rpc.go @@ -83,9 +83,17 @@ type Entry struct { // these fields are only valid based on payload type and if payload could be successfully parsed and will by default be equal to zero values Sender string `json:"sender"` - // SenderVerified is true ONLY when attribution is protocol-pinned (ring size 2, - // where the counterparty is necessarily the sender). For ring size > 2 the sender - // chose the unauthenticated attribution byte and entry.Sender MUST NOT be trusted. + // SenderVerified is true when entry.Sender can be trusted: either attribution is + // protocol-pinned (ring size 2, where the counterparty is necessarily the sender) + // or this wallet authored the transaction (an outgoing/self send, certain at any + // ring size). For an incoming ring size > 2 transfer the sender chose the + // unauthenticated attribution byte and entry.Sender MUST NOT be trusted. + // + // Renderer truth table (RingSize, SenderVerified, Sender): + // ring 2 / true / addr -> trusted counterparty; show it + // self / true / own addr -> our own send; show it + // ring>2 / false / "" -> withheld (scrubbed); show "unknown (unverifiable, ring N)" + // any / false / "" + PayloadError != "" -> decode failed (distinct from a scrub) SenderVerified bool `json:"sender_verified"` // RingSize is the ring size of the payload this entry was decoded from (0 if unknown). RingSize uint64 `json:"ringsize"` diff --git a/walletapi/attribution_scrub_behavior_test.go b/walletapi/attribution_scrub_behavior_test.go new file mode 100644 index 00000000..faedb311 --- /dev/null +++ b/walletapi/attribution_scrub_behavior_test.go @@ -0,0 +1,256 @@ +// Copyright 2017-2021 DERO Project. All rights reserved. +// Use of this source code in any form is governed by RESEARCH license. +// license can be found in the LICENSE file. +// GPG: 0F39 E425 8C65 3947 702A 8234 08B2 0360 A03A 9DE8 +// +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY +// EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF +// MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL +// THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS +// INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, +// STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +package walletapi + +import ( + "fmt" + "os" + "path/filepath" + "testing" + "time" + + "github.com/deroproject/derohe/blockchain" + "github.com/deroproject/derohe/config" + "github.com/deroproject/derohe/cryptography/crypto" + "github.com/deroproject/derohe/globals" + "github.com/deroproject/derohe/rpc" + "github.com/deroproject/derohe/transaction" +) + +// Test_Attribution_Scrub_Behavior is the behavioral companion to the source-level +// grep guard (Test_AttributionExportGuard_UnverifiedSlotByteScrubbed). The grep guard +// proves the scrub TEXT is present at both decode sites; this test drives the REAL +// receiver decode path end-to-end on a simulated chain and asserts the scrub's effect +// on the exported Entry fields. A logically-wrong-but-textually-present scrub (inverted +// condition, wrong index, wrong RingSize comparison) passes the grep guard but FAILS here. +// +// It asserts the contract for both cases: +// - ring == 2 (verified, structural): entry.Sender is populated, entry.Data[0] (the +// attribution slot byte) is preserved, SenderVerified == true. +// - ring > 2 (unverified, sender-chosen byte): entry.Sender == "", entry.Data[0] == 0, +// SenderVerified == false. The decrypted Payload (parsed args) survives in both cases. +func Test_Attribution_Scrub_Behavior(t *testing.T) { + + time.Sleep(time.Millisecond) + + Initialize_LookupTable(1, 1<<17) + + wsrc_temp_db := filepath.Join(os.TempDir(), "scrub_test_wallet_src.db") + wdst_temp_db := filepath.Join(os.TempDir(), "scrub_test_wallet_dst.db") + + os.Remove(wsrc_temp_db) + os.Remove(wdst_temp_db) + + wsrc, err := Create_Encrypted_Wallet_From_Recovery_Words(wsrc_temp_db, "QWER", "sequence atlas unveil summon pebbles tuesday beer rudely snake rockets different fuselage woven tagged bested dented vegan hover rapid fawns obvious muppet randomly seasons randomly") + if err != nil { + t.Fatalf("Cannot create encrypted wallet, err %s", err) + } + + wdst, err := Create_Encrypted_Wallet_From_Recovery_Words(wdst_temp_db, "QWER", "Dekade Spagat Bereich Radclub Yeti Dialekt Unimog Nomade Anlage Hirte Besitz Märzluft Krabbe Nabel Halsader Chefarzt Hering tauchen Neuerung Reifen Umgang Hürde Alchimie Amnesie Reifen") + if err != nil { + t.Fatalf("Cannot create encrypted wallet, err %s", err) + } + + wgenesis, err := Create_Encrypted_Wallet_From_Recovery_Words(wdst_temp_db, "QWER", "perfil lujo faja puma favor pedir detalle doble carbón neón paella cuarto ánimo cuento conga correr dental moneda león donar entero logro realidad acceso doble") + if err != nil { + t.Fatalf("Cannot create encrypted wallet, err %s", err) + } + + // fix genesis tx and genesis tx hash + genesis_tx := transaction.Transaction{Transaction_Prefix: transaction.Transaction_Prefix{Version: 1, Value: 2012345}} + copy(genesis_tx.MinerAddress[:], wgenesis.account.Keys.Public.EncodeCompressed()) + + config.Testnet.Genesis_Tx = fmt.Sprintf("%x", genesis_tx.Serialize()) + config.Mainnet.Genesis_Tx = fmt.Sprintf("%x", genesis_tx.Serialize()) + + genesis_block := blockchain.Generate_Genesis_Block() + config.Testnet.Genesis_Block_Hash = genesis_block.GetHash() + config.Mainnet.Genesis_Block_Hash = genesis_block.GetHash() + + chain, rpcserver, params := simulator_chain_start() + _ = params + + defer func() { + simulator_chain_stop(chain, rpcserver) + wsrc.Close_Encrypted_Wallet() + wdst.Close_Encrypted_Wallet() + os.Remove(wsrc_temp_db) + os.Remove(wdst_temp_db) + }() + + globals.Arguments["--daemon-address"] = rpcport + + go Keep_Connectivity() + + if err := chain.Add_TX_To_Pool(wsrc.GetRegistrationTX()); err != nil { + t.Fatalf("Cannot add regtx to pool err %s", err) + } + if err := chain.Add_TX_To_Pool(wdst.GetRegistrationTX()); err != nil { + t.Fatalf("Cannot add regtx to pool err %s", err) + } + + for i := 0; i < 5; i++ { + simulator_chain_mineblock(chain, wgenesis.GetAddress(), t) + } + + wgenesis.SetDaemonAddress(rpcport) + wsrc.SetDaemonAddress(rpcport) + wdst.SetDaemonAddress(rpcport) + wgenesis.SetOnlineMode() + wsrc.SetOnlineMode() + wdst.SetOnlineMode() + + time.Sleep(time.Second * 2) + if err = wsrc.Sync_Wallet_Memory_With_Daemon(); err != nil { + t.Fatalf("Wallet sync error err %s", err) + } + if err = wdst.Sync_Wallet_Memory_With_Daemon(); err != nil { + t.Fatalf("Wallet sync error err %s", err) + } + + testPayload := rpc.Arguments{ + {Name: rpc.RPC_COMMENT, DataType: rpc.DataString, Value: "scrub probe"}, + {Name: rpc.RPC_DESTINATION_PORT, DataType: rpc.DataUint64, Value: uint64(123456789)}, + } + + // send one transfer at each ring size: index 0 = ring 2 (verified control), + // index 1 = ring 4 (unverified, triggers the scrub). + ringSizes := []int{2, 4} + + for idx, rs := range ringSizes { + wsrc.Sync_Wallet_Memory_With_Daemon() + wdst.Sync_Wallet_Memory_With_Daemon() + + wsrc.account.Ringsize = rs + + // The honest pre-scrub attribution byte is witness_index[1] (transaction_build.go:187) — + // the RECEIVER's OWN ring slot index. At ring 4 the receiver lands in slot 0 in 25% of + // valid permutations, in which case the honest byte is ALREADY 0x00 and a post-decode + // entry.Data[0]==0 assertion passes EVEN IF THE SCRUB IS BROKEN (coincidence, not the + // scrub). To give the ring>2 assertion real teeth we rebuild (re-randomizing the ring) + // until the receiver lands in a NON-ZERO slot: then entry.Data[0]==0 can ONLY be the + // scrub's doing. Ring 2 is exempt — its byte is protocol-pinned, not scrub-asserted. + var tx *transaction.Transaction + recvSlot := -1 + for attempt := 0; attempt < 64; attempt++ { + var berr error + tx, berr = wsrc.TransferPayload0([]rpc.Transfer{{Destination: wdst.GetAddress().String(), Amount: 90000, Payload_RPC: testPayload}}, 0, false, rpc.Arguments{}, 10000, false) + if berr != nil { + t.Fatalf("ring %d (send %d): cannot create transaction, err %s", rs, idx, berr) + } + if got := len(tx.Payloads[0].Statement.Publickeylist); got != rs { + t.Fatalf("ring %d (send %d): built ring size %d, expected %d", rs, idx, got, rs) + } + recvSlot = -1 + dstKey := wdst.account.Keys.Public.EncodeCompressed() + for i, p := range tx.Payloads[0].Statement.Publickeylist { + if string((*crypto.Point)(p).EncodeCompressed()) == string(dstKey) { + recvSlot = i + break + } + } + if recvSlot < 0 { + t.Fatalf("ring %d (send %d): receiver pubkey not found in built ring", rs, idx) + } + if rs == 2 || recvSlot != 0 { + break // ring 2 needs no teeth; ring>2 needs a non-zero honest byte + } + } + if rs > 2 && recvSlot == 0 { + t.Fatalf("ring %d (send %d): could not build a ring with the receiver in a non-zero slot "+ + "after 64 attempts; the entry.Data[0]==0 assertion would have no teeth", rs, idx) + } + + var dtx transaction.Transaction + dtx.Deserialize(tx.Serialize()) + + simulator_chain_mineblock(chain, wgenesis.GetAddress(), t) + wsrc.Sync_Wallet_Memory_With_Daemon() + wdst.Sync_Wallet_Memory_With_Daemon() + + if err := chain.Add_TX_To_Pool(&dtx); err != nil { + t.Fatalf("ring %d (send %d): cannot add transfer to pool, err %s", rs, idx, err) + } + + simulator_chain_mineblock(chain, wgenesis.GetAddress(), t) + wgenesis.Sync_Wallet_Memory_With_Daemon() + } + + time.Sleep(time.Second) + wdst.Sync_Wallet_Memory_With_Daemon() + + minHeight := uint64(0) + maxHeight := uint64(chain.Get_Height()) + 1 + + dstEntries := wdst.Show_Transfers(crypto.ZEROHASH, false, true, false, minHeight, maxHeight, "", "", 0, 0) + if len(dstEntries) != len(ringSizes) { + t.Fatalf("receiver expected %d transfers, got %d", len(ringSizes), len(dstEntries)) + } + + var sawVerified, sawUnverified bool + for _, e := range dstEntries { + // the decrypted payload must survive intact regardless of scrub: the comment + // arg parses in both cases (scrub only touches the leading attribution slot byte). + args, err := e.ProcessPayload() + if err != nil { + t.Fatalf("ring %d: payload did not parse after decode: %s", e.RingSize, err) + } + if !args.HasValue(rpc.RPC_COMMENT, rpc.DataString) { + t.Fatalf("ring %d: decrypted payload lost its comment arg after scrub", e.RingSize) + } + + switch e.RingSize { + case 2: + sawVerified = true + if !e.SenderVerified { + t.Fatalf("ring 2: SenderVerified must be true (structural attribution)") + } + if e.Sender == "" { + t.Fatalf("ring 2: verified Sender must be populated, got empty") + } + if len(e.Data) == 0 { + t.Fatalf("ring 2: entry.Data unexpectedly empty") + } + // ring-2 attribution byte is protocol-pinned and intentionally preserved: + // the receiver overrides sender_idx to the counterparty slot, so a zero + // here would only be coincidental. The contract is that it is NOT scrubbed, + // which is proven by Sender being populated above. + default: // ring > 2 + sawUnverified = true + if e.SenderVerified { + t.Fatalf("ring %d: SenderVerified must be false for sender-chosen attribution", e.RingSize) + } + if e.Sender != "" { + t.Fatalf("ring %d: unverified Sender must be scrubbed to empty, got %q", e.RingSize, e.Sender) + } + if len(e.Data) == 0 { + t.Fatalf("ring %d: entry.Data unexpectedly empty", e.RingSize) + } + if e.Data[0] != 0x00 { + t.Fatalf("ring %d: attribution slot byte entry.Data[0] must be zeroed, got 0x%02x — "+ + "it re-derives the claimed sender via the public Publickeylist", e.RingSize, e.Data[0]) + } + } + } + + if !sawVerified { + t.Fatalf("no ring-2 (verified) transfer observed; control case did not run") + } + if !sawUnverified { + t.Fatalf("no ring>2 (unverified) transfer observed; scrub case did not run") + } +} diff --git a/walletapi/daemon_communication.go b/walletapi/daemon_communication.go index 501c3f2d..7fc73992 100644 --- a/walletapi/daemon_communication.go +++ b/walletapi/daemon_communication.go @@ -985,6 +985,10 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err addr := rpc.NewAddressFromKeys((*crypto.Point)(w.account.Keys.Public.G1())) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() + // this wallet authored the tx, so the sender (ourselves) is certain + // at any ring size; mark it verified so consumers do not distrust our own sends. + entry.RingSize = uint64(tx.Payloads[t].Statement.RingSize) + entry.SenderVerified = true entry.Payload = append(entry.Payload, tx.Payloads[t].RPCPayload[1:]...) entry.Data = append(entry.Data, tx.Payloads[t].RPCPayload[:]...) @@ -1010,6 +1014,10 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err addr := rpc.NewAddressFromKeys((*crypto.Point)(w.account.Keys.Public.G1())) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() + // this wallet authored the tx, so the sender (ourselves) is certain + // at any ring size; mark it verified so consumers do not distrust our own sends. + entry.RingSize = uint64(tx.Payloads[t].Statement.RingSize) + entry.SenderVerified = true entry.Payload = append(entry.Payload, payload[1:]...) entry.Data = append(entry.Data, payload...) @@ -1093,7 +1101,7 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err } entry.Payload = append(entry.Payload, tx.Payloads[t].RPCPayload[1:]...) - entry.Data = append(entry.Data, exported_payload[:]...) + entry.Data = append(entry.Data, exported_payload...) args, _ := entry.ProcessPayload() _ = args From 6446d66378722376cd8137143cc0ee84878f1551 Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Thu, 2 Jul 2026 14:28:17 -0400 Subject: [PATCH 5/5] fix(rpc): render unverifiable sender per truth table + document non-retroactive scrub (review #1/#4/#5/#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6 — Entry.String() printed "Sender: %s" unconditionally: a scrubbed ring>2 transfer rendered a bare "Sender: " line that reads as a decode bug — the exact "looks broken" outcome the design meant to avoid, contradicting the truth table documented on SenderVerified in the same file. The Sender line now follows the table: verified -> the address; scrubbed (no payload error) -> "unknown (unverifiable, ring size N)"; decode failure -> a distinct "unknown (payload decode failed)". Blast radius: two CLI debug dumps (%+v in show_transfers error paths); JSON marshalling untouched. Pinned by rpc/wallet_rpc_test.go (offline, sub-second): all three arms, plus forbids the bare line and scrub/decode-failure conflation. #4/#5 — the non-retroactive caveat lived only in commit 4e9de56's message. Now in the docs consumers read: SenderVerified's doc comment states the scrub is decode-time only (pre-upgrade entries keep their guessed Sender and unscrubbed Data[0], served as-is by Get_Transfers; the guarantee holds only for blocks decoded post-upgrade), the upgrade is one-way for the privacy property, and pre-upgrade entries deserialize SenderVerified=false, RingSize=0 — indistinguishable from a scrub without re-decoding, so a migration MUST re-derive RingSize from chain data (a read-path scrub keyed on persisted fields would wrongly blank the wallet's own verified sends — why the rescan is a follow-up, not this commit). RingSize's doc gains the "0 = pre-upgrade/unknown, not a real ring" note; the GetTransfers handler doc points RPC consumers at the contract. #1 hardening — both decode arms bounded sender_idx against Statement.RingSize while indexing Statement.Publickeylist. Safe today (len(Publickeylist)==RingSize holds at that point), but the guard now bounds against the slice actually indexed, so it survives any future break of that invariant. Existing pins green: the scrub-behavior sim test and the source-grep guard (its regexes key on the scrub lines, untouched here). --- rpc/wallet_rpc.go | 23 ++++++++++- rpc/wallet_rpc_test.go | 52 +++++++++++++++++++++++++ walletapi/daemon_communication.go | 4 +- walletapi/rpcserver/rpc_gettransfers.go | 4 ++ 4 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 rpc/wallet_rpc_test.go diff --git a/rpc/wallet_rpc.go b/rpc/wallet_rpc.go index af2acdab..29e4a779 100644 --- a/rpc/wallet_rpc.go +++ b/rpc/wallet_rpc.go @@ -94,8 +94,19 @@ type Entry struct { // self / true / own addr -> our own send; show it // ring>2 / false / "" -> withheld (scrubbed); show "unknown (unverifiable, ring N)" // any / false / "" + PayloadError != "" -> decode failed (distinct from a scrub) + // + // NON-RETROACTIVE: the scrub runs at DECODE time only. Entries persisted before the + // wallet upgraded keep their guessed Sender and unscrubbed Data[0], and are served + // as-is (Get_Transfers included) — the guarantee holds only for blocks decoded + // post-upgrade. The upgrade is one-way for the privacy property: pre-upgrade entries + // deserialize SenderVerified=false, RingSize=0 (zero values), which is + // indistinguishable from a scrub without re-decoding — so a migration MUST re-derive + // RingSize from chain data, never trust the persisted 0 (a read-path scrub keyed on + // these fields would wrongly blank the wallet's own verified sends). SenderVerified bool `json:"sender_verified"` - // RingSize is the ring size of the payload this entry was decoded from (0 if unknown). + // RingSize is the ring size of the payload this entry was decoded from. 0 means + // unknown — typically an entry persisted before the attribution upgrade — NOT a real + // ring; see the non-retroactive caveat on SenderVerified. RingSize uint64 `json:"ringsize"` DestinationPort uint64 `json:"dstport"` SourcePort uint64 `json:"srcport"` @@ -122,7 +133,15 @@ func (e Entry) String() string { if !e.Coinbase { fmt.Fprintf(&b, "PayloadType : %d\n", e.PayloadType) if e.PayloadType == 0 { - fmt.Fprintf(&b, "Sender: %s\n", e.Sender) + // the Sender line follows the truth table on SenderVerified above: a scrubbed + // attribution must read as a deliberate refusal, distinct from a decode failure. + if e.SenderVerified { + fmt.Fprintf(&b, "Sender: %s\n", e.Sender) + } else if e.PayloadError == "" { + fmt.Fprintf(&b, "Sender: unknown (unverifiable, ring size %d)\n", e.RingSize) + } else { + fmt.Fprintf(&b, "Sender: unknown (payload decode failed)\n") + } if e.PayloadError == "" { args, _ := e.ProcessPayload() fmt.Fprintf(&b, "DestPort: %016x\n", e.DestinationPort) diff --git a/rpc/wallet_rpc_test.go b/rpc/wallet_rpc_test.go new file mode 100644 index 00000000..b8754001 --- /dev/null +++ b/rpc/wallet_rpc_test.go @@ -0,0 +1,52 @@ +// Copyright 2017-2021 DERO Project. All rights reserved. +// Use of this source code in any form is governed by RESEARCH license. +// license can be found in the LICENSE file. + +package rpc + +import ( + "strings" + "testing" +) + +// Test_Entry_String_SenderTruthTable pins Entry.String()'s Sender line to the +// renderer truth table documented on SenderVerified (PR #21 review finding #6): +// a scrubbed attribution must read as a deliberate refusal — never a bare +// "Sender: " line that looks like a decode bug — and stay distinct from an +// actual decode failure. +func Test_Entry_String_SenderTruthTable(t *testing.T) { + cases := []struct { + name string + e Entry + want string + forbid string + }{ + { + name: "verified sender is shown", + e: Entry{Incoming: true, PayloadType: 0, Sender: "deto1qyexampleaddress", SenderVerified: true, RingSize: 2}, + want: "Sender: deto1qyexampleaddress\n", + }, + { + name: "scrubbed ring>2 renders unverifiable with ring size", + e: Entry{Incoming: true, PayloadType: 0, Sender: "", SenderVerified: false, RingSize: 4}, + want: "Sender: unknown (unverifiable, ring size 4)\n", + forbid: "Sender: \n", + }, + { + name: "decode failure renders distinctly from a scrub", + e: Entry{Incoming: true, PayloadType: 0, Sender: "", SenderVerified: false, RingSize: 4, PayloadError: "short payload"}, + want: "Sender: unknown (payload decode failed)\n", + forbid: "unverifiable", + }, + } + + for _, c := range cases { + out := c.e.String() + if !strings.Contains(out, c.want) { + t.Fatalf("%s: rendered output missing %q:\n%s", c.name, c.want, out) + } + if c.forbid != "" && strings.Contains(out, c.forbid) { + t.Fatalf("%s: rendered output must not contain %q:\n%s", c.name, c.forbid, out) + } + } +} diff --git a/walletapi/daemon_communication.go b/walletapi/daemon_communication.go index 7fc73992..54d842d9 100644 --- a/walletapi/daemon_communication.go +++ b/walletapi/daemon_communication.go @@ -1079,7 +1079,7 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err } } - if sender_idx < uint(tx.Payloads[t].Statement.RingSize) { // off-by-one fix: valid indices are 0..RingSize-1 + if sender_idx < uint(len(tx.Payloads[t].Statement.Publickeylist)) { // bound against the slice actually indexed (survives any future break of the len(Publickeylist)==RingSize invariant); valid indices are 0..len-1 addr := rpc.NewAddressFromKeys((*crypto.Point)(tx.Payloads[t].Statement.Publickeylist[sender_idx])) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() @@ -1133,7 +1133,7 @@ func (w *Wallet_Memory) synchistory_block(scid crypto.Hash, topo int64) (err err } } - if sender_idx < uint(tx.Payloads[t].Statement.RingSize) { // off-by-one fix: valid indices are 0..RingSize-1 + if sender_idx < uint(len(tx.Payloads[t].Statement.Publickeylist)) { // bound against the slice actually indexed (survives any future break of the len(Publickeylist)==RingSize invariant); valid indices are 0..len-1 addr := rpc.NewAddressFromKeys((*crypto.Point)(tx.Payloads[t].Statement.Publickeylist[sender_idx])) addr.Mainnet = w.GetNetwork() entry.Sender = addr.String() diff --git a/walletapi/rpcserver/rpc_gettransfers.go b/walletapi/rpcserver/rpc_gettransfers.go index c835723c..f10f2341 100644 --- a/walletapi/rpcserver/rpc_gettransfers.go +++ b/walletapi/rpcserver/rpc_gettransfers.go @@ -24,6 +24,10 @@ import ( "github.com/deroproject/derohe/rpc" ) +// GetTransfers serves persisted entries as-is. Consumers reading sender / +// sender_verified / ringsize MUST honor the rpc.Entry.SenderVerified contract, +// including its non-retroactive caveat: entries persisted before the attribution +// upgrade are NOT re-scrubbed and deserialize SenderVerified=false, RingSize=0. func GetTransfers(ctx context.Context, p rpc.Get_Transfers_Params) (result rpc.Get_Transfers_Result, err error) { defer func() { // safety so if anything wrong happens, we return error if r := recover(); r != nil {