From 737b3f64c354728be50ae483b5697179dae80b53 Mon Sep 17 00:00:00 2001 From: AMATH <116212274+amathxbt@users.noreply.github.com> Date: Sat, 4 Jul 2026 09:11:08 +0100 Subject: [PATCH] fix(contract): stop swallowing FSM errors and avoid conflicting write ops in DeliverMessageSend Two issues in DeliverMessageSend(): 1. When the StateRead response carries an FSM-level error (response.Error != nil), the handler returned Error: err instead of Error: response.Error. Since err was already confirmed nil a few lines above, this silently discarded the real error and let the function fall through to unmarshal zero-value account bytes as if the read had simply returned empty state. CheckTx already handles this correctly (err = resp.Error); DeliverMessageSend did not. Fixed to propagate response.Error. 2. For a self-transfer that drains the balance to exactly zero, fromKey == toKey, but the write batch included both a Set for toKey and a Delete for fromKey -- i.e. a Set and a Delete for the identical key in one batch. That is undefined/apply-order-dependent in the underlying store, which is a state-determinism risk across nodes. Fixed to only include the 'to' Set when it targets a distinct account from 'from'. --- contract/contract.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/contract/contract.go b/contract/contract.go index 171d220..25e7dab 100644 --- a/contract/contract.go +++ b/contract/contract.go @@ -132,8 +132,11 @@ func (c *Contract) DeliverMessageSend(msg *MessageSend, fee uint64) *PluginDeliv return &PluginDeliverResponse{Error: err} } // ensure no error fsm error + // NOTE: must return response.Error here, not err (err is nil at this point since it was + // already checked above) — otherwise a failed state read is silently discarded and the + // function falls through to unmarshal zero-value bytes as if the accounts didn't error. if response.Error != nil { - return &PluginDeliverResponse{Error: err} + return &PluginDeliverResponse{Error: response.Error} } // get the from bytes and to bytes for _, resp := range response.Results { @@ -189,11 +192,16 @@ func (c *Contract) DeliverMessageSend(msg *MessageSend, fee uint64) *PluginDeliv var resp *PluginStateWriteResponse // if the from account is drained - delete the from account if from.Amount == 0 { + sets := []*PluginSetOp{{Key: feePoolKey, Value: feePoolBytes}} + // for a self-transfer that drains the balance to zero, fromKey == toKey. Including a + // Set and a Delete for the identical key in the same write batch is undefined/order + // dependent in the underlying store, which risks non-deterministic state across nodes. + // Only include the 'to' Set when it targets a distinct account. + if !bytes.Equal(fromKey, toKey) { + sets = append(sets, &PluginSetOp{Key: toKey, Value: toBytes}) + } resp, err = c.plugin.StateWrite(c, &PluginStateWriteRequest{ - Sets: []*PluginSetOp{ - {Key: feePoolKey, Value: feePoolBytes}, - {Key: toKey, Value: toBytes}, - }, + Sets: sets, Deletes: []*PluginDeleteOp{{Key: fromKey}}, }) } else {