Conversation
…atform/gowaves into ethereum-transaction-refactoring
| return EthereumTransferWavesKind, nil | ||
| } | ||
|
|
||
| selectorBytes := data |
There was a problem hiding this comment.
This is unnecessary variable
| if len(data) < ethabi.SelectorSize { | ||
| return 0, errors.Errorf("length of data from ethereum transaction is less than %d", ethabi.SelectorSize) | ||
| } | ||
| selector, err := ethabi.NewSelectorFromBytes(selectorBytes[:ethabi.SelectorSize]) |
There was a problem hiding this comment.
data[:ethabi.SelectorSize] does the same. Maybe you meant selectorBytes := data[:ethabi.SelectorSize]. Correct me if I'm wrong
| if err != nil { | ||
| return nil, errors.Errorf("failed to parse ethereum data") | ||
| } | ||
| if len(decodedData.Inputs) != ethabi.NumberOfERC20TransferArguments { |
There was a problem hiding this comment.
This check is unnecessary. ethabi.GetERC20TransferArguments does the same check inside.
| case *EthereumTransferAssetsErc20TxKind: | ||
| return nil | ||
| case *EthereumInvokeScriptTxKind: | ||
| return nil |
There was a problem hiding this comment.
Merge this cases or rewrite whole switch to if...else.
| res := new(big.Int).Div(tx.inner.value(), big.NewInt(int64(DiffEthWaves))) | ||
| tx.value = res.Int64() | ||
|
|
There was a problem hiding this comment.
You should handle it res doesn't fit into int64 value even in tests.
| // Waves representation | ||
| TxKind EthereumTransactionKind | ||
| value int64 | ||
| chainID int64 |
There was a problem hiding this comment.
Use proto.Scheme type instead of int64. And for WAVES network correct naming is sheme, not chainID.
| // Ethereum representation | ||
| senderPK atomic.Value // *EthereumPublicKey | ||
| inner EthereumTxData // | ||
|
|
||
| // Waves representation | ||
| TxKind EthereumTransactionKind |
There was a problem hiding this comment.
Use different naming for Ethereum representation fields and Waves representation fields. I think it would be better if you create special struct for valid waves representation.
| return nil, errs.NewFeeValidation("insufficient fee") | ||
| } | ||
| // too many waves (this check doesn't exist in scala) | ||
| // TODO I'm not sure this is should be checked for all eth tx kinds. Only for transfer waves kind |
There was a problem hiding this comment.
It's valid. Negative wavelets amount is invalid. It must be zero or greater.
| func (tx *EthereumTransaction) ChainId() int64 { | ||
| return tx.chainID | ||
| } |
There was a problem hiding this comment.
You should change return type to proto.Scheme. And I think it would be better if you rename this method to Scheme().
| func (tx *EthereumTransaction) Value() *big.Int { return copyBigInt(tx.inner.value()) } | ||
| func (tx *EthereumTransaction) Value() int64 { return tx.value } |
There was a problem hiding this comment.
Comment says that Value returns the ether amount of the transaction.. You return wavelets
| res := new(big.Int).Div(tx.inner.value(), big.NewInt(int64(DiffEthWaves))) | ||
| if ok := res.IsInt64(); !ok { | ||
| return errors.Errorf("failed to convert amount from ethreum transaction (big int) to int64. value is %d", tx.Value()) | ||
| } |
There was a problem hiding this comment.
You can easily use previous version for of EthereumWeiToWavelet function for conversion.
nickeskov
added
do not merge
2 participants
No description provided.