Qi Address Scope Check #1604
Qi Address Scope Check #1604
Conversation
jdowning100
requested review from
wizeguyy,
kiltsonfire and
gameofpointers
as code owners
jdowning100
force-pushed
the
qi-address-check
branch
from
498b1af to
7cf8ae2
Compare
| @@ -309,7 +309,9 @@ func (p *StateProcessor) Process(block *types.WorkObject, etxSet *types.EtxSet) | |||
| return nil, nil, nil, nil, 0, fmt.Errorf("etx %032x emits UTXO with value %d greater than max denomination", tx.Hash(), tx.Value().Int64()) | |||
| } | |||
| // There are no more checks to be made as the ETX is worked so add it to the set | |||
| statedb.CreateUTXO(tx.OriginatingTxHash(), tx.ETXIndex(), types.NewUtxoEntry(types.NewTxOut(uint8(tx.Value().Int64()), tx.To().Bytes()))) | |||
| if err := statedb.CreateUTXO(tx.OriginatingTxHash(), tx.ETXIndex(), types.NewUtxoEntry(types.NewTxOut(uint8(tx.Value().Int64()), tx.To().Bytes()))); err != nil { | |||
There was a problem hiding this comment.
Going from big.Int to Int64 to uint8 seems sus. Can you confirm our values never exceed uint8?
There was a problem hiding this comment.
The check for this is above:
if tx.Value().Int64() > types.MaxDenomination { // sanity check
return nil, nil, nil, nil, 0, fmt.Errorf("etx %032x emits UTXO with value %d greater than max denomination", tx.Hash(), tx.Value().Int64())
There was a problem hiding this comment.
Ok since we do the overflow sanity check is there anywhere we check that the value is strictly above 0?
There was a problem hiding this comment.
Making it a uint64 would fix it too. That would simultaneously check that the value is below the max denomination and that it was positive.
There was a problem hiding this comment.
There was a problem hiding this comment.
Updated the conversion PR to use big.Int.Uint64(). Please review that PR instead of this one as that PR includes this one.
|
Closed in favor of #1559 |
@dominant-strategies/core-dev
I am quite sure the check is redundant, but check if an address is in Qi ledger scope and internal shard scope before adding new utxo to set.
#1400