diff --git a/rpc/blockchain.go b/rpc/blockchain.go index 284bff5af7..c9a6a13138 100644 --- a/rpc/blockchain.go +++ b/rpc/blockchain.go @@ -464,7 +464,11 @@ func (s *PublicBlockchainService) GetBlockReceipts( r, err = v2.NewReceipt(tx, blockHash, block.NumberU64(), index, rmap[tx.Hash()]) case Eth: if tx, ok := tx.(*types.Transaction); ok { - r, err = v2.NewReceipt(tx, blockHash, block.NumberU64(), index, rmap[tx.Hash()]) + from, err := tx.SenderAddress() + if err != nil { + return nil, err + } + r, err = eth.NewReceipt(from, tx.ConvertToEth(), blockHash, block.NumberU64(), index, rmap[tx.Hash()]) } default: return nil, ErrUnknownRPCVersion diff --git a/rpc/eth/types.go b/rpc/eth/types.go index 2767984cdd..c51d604b2d 100644 --- a/rpc/eth/types.go +++ b/rpc/eth/types.go @@ -74,19 +74,9 @@ type Transaction struct { // representation, with the given location metadata set (if available). // Note that all txs on Harmony are replay protected (post EIP155 epoch). func NewTransaction( - tx *types.EthTransaction, blockHash common.Hash, + from common.Address, tx *types.EthTransaction, blockHash common.Hash, blockNumber uint64, timestamp uint64, index uint64, ) (*Transaction, error) { - from := common.Address{} - var err error - if tx.IsEthCompatible() { - from, err = tx.SenderAddress() - } else { - from, err = tx.ConvertToHmy().SenderAddress() - } - if err != nil { - return nil, err - } v, r, s := tx.RawSignatureValues() result := &Transaction{ @@ -111,59 +101,10 @@ func NewTransaction( return result, nil } -// NewReceipt returns the RPC data for a new receipt. It is unused at the moment. -func NewReceipt(tx *types.EthTransaction, blockHash common.Hash, blockNumber, blockIndex uint64, receipt *types.Receipt) (map[string]interface{}, error) { - senderAddr, err := tx.SenderAddress() - if err != nil { - return nil, err - } - +// NewReceipt returns the RPC data for a new receipt +func NewReceipt(senderAddr common.Address, tx *types.EthTransaction, blockHash common.Hash, blockNumber, blockIndex uint64, receipt *types.Receipt) (map[string]interface{}, error) { ethTxHash := tx.Hash() - for i, _ := range receipt.Logs { - // Override log txHash with receipt's - receipt.Logs[i].TxHash = ethTxHash - } - - fields := map[string]interface{}{ - "blockHash": blockHash, - "blockNumber": hexutil.Uint64(blockNumber), - "transactionHash": ethTxHash, - "transactionIndex": hexutil.Uint64(blockIndex), - "from": senderAddr, - "to": tx.To(), - "gasUsed": hexutil.Uint64(receipt.GasUsed), - "cumulativeGasUsed": hexutil.Uint64(receipt.CumulativeGasUsed), - "contractAddress": nil, - "logs": receipt.Logs, - "logsBloom": receipt.Bloom, - } - - // Assign receipt status or post state. - if len(receipt.PostState) > 0 { - fields["root"] = hexutil.Bytes(receipt.PostState) - } else { - fields["status"] = hexutil.Uint(receipt.Status) - } - if receipt.Logs == nil { - fields["logs"] = [][]*types.Log{} - } - // If the ContractAddress is 20 0x0 bytes, assume it is not a contract creation - if receipt.ContractAddress != (common.Address{}) { - fields["contractAddress"] = receipt.ContractAddress - } - - return fields, nil -} - -// NewReceiptFromTransaction returns the RPC data for a new receipt. It is unused at the moment. -func NewReceiptFromTransaction(tx *types.Transaction, blockHash common.Hash, blockNumber, blockIndex uint64, receipt *types.Receipt) (map[string]interface{}, error) { - senderAddr, err := tx.SenderAddress() - if err != nil { - return nil, err - } - - ethTxHash := tx.Hash() - for i, _ := range receipt.Logs { + for i := range receipt.Logs { // Override log txHash with receipt's receipt.Logs[i].TxHash = ethTxHash } @@ -253,7 +194,11 @@ func blockWithFullTxFromBlock(b *types.Block) (*BlockWithFullTx, error) { } for idx, tx := range b.Transactions() { - fmtTx, err := NewTransaction(tx.ConvertToEth(), b.Hash(), b.NumberU64(), b.Time().Uint64(), uint64(idx)) + from, err := tx.SenderAddress() + if err != nil { + return nil, err + } + fmtTx, err := NewTransaction(from, tx.ConvertToEth(), b.Hash(), b.NumberU64(), b.Time().Uint64(), uint64(idx)) if err != nil { return nil, err } @@ -270,5 +215,10 @@ func NewTransactionFromBlockIndex(b *types.Block, index uint64) (*Transaction, e "tx index %v greater than or equal to number of transactions on block %v", index, b.Hash().String(), ) } - return NewTransaction(txs[index].ConvertToEth(), b.Hash(), b.NumberU64(), b.Time().Uint64(), index) + tx := txs[index].ConvertToEth() + from, err := tx.SenderAddress() + if err != nil { + return nil, err + } + return NewTransaction(from, tx, b.Hash(), b.NumberU64(), b.Time().Uint64(), index) } diff --git a/rpc/pool.go b/rpc/pool.go index c0d4858c2e..ee4e34828f 100644 --- a/rpc/pool.go +++ b/rpc/pool.go @@ -253,7 +253,14 @@ func (s *PublicPoolService) PendingTransactions( continue // Legacy behavior is to not return error here } case Eth: - tx, err = eth.NewTransaction(plainTx.ConvertToEth(), common.Hash{}, 0, 0, 0) + from, err := plainTx.SenderAddress() + if err != nil { + utils.Logger().Debug(). + Err(err). + Msgf("%v error at %v", LogTag, "PendingTransactions") + continue // Legacy behavior is to not return error here + } + tx, err = eth.NewTransaction(from, plainTx.ConvertToEth(), common.Hash{}, 0, 0, 0) if err != nil { utils.Logger().Debug(). Err(err). diff --git a/rpc/transaction.go b/rpc/transaction.go index 53f8f47675..4b45045851 100644 --- a/rpc/transaction.go +++ b/rpc/transaction.go @@ -236,7 +236,13 @@ func (s *PublicTransactionService) newRPCTransaction(tx *types.Transaction, bloc } return NewStructuredResponse(tx) case Eth: - tx, err := eth.NewTransaction(tx.ConvertToEth(), blockHash, blockNumber, timestamp, index) + // calculate SenderAddress before ConvertToEth + senderAddr, err := tx.SenderAddress() + if err != nil { + DoMetricRPCQueryInfo(GetTransactionByHash, FailedNumber) + return nil, err + } + tx, err := eth.NewTransaction(senderAddr, tx.ConvertToEth(), blockHash, blockNumber, timestamp, index) if err != nil { DoMetricRPCQueryInfo(GetTransactionByHash, FailedNumber) return nil, err @@ -763,7 +769,12 @@ func (s *PublicTransactionService) GetTransactionReceipt( return NewStructuredResponse(RPCReceipt) case Eth: if tx != nil { - RPCReceipt, err = eth.NewReceiptFromTransaction(tx, blockHash, blockNumber, index, receipt) + // calculate SenderAddress before ConvertToEth + senderAddr, err := tx.SenderAddress() + if err != nil { + return nil, err + } + RPCReceipt, err = eth.NewReceipt(senderAddr, tx.ConvertToEth(), blockHash, blockNumber, index, receipt) } if err != nil { return nil, err diff --git a/rpc/v2/types.go b/rpc/v2/types.go index 76f36ad40e..c9abbead05 100644 --- a/rpc/v2/types.go +++ b/rpc/v2/types.go @@ -204,20 +204,20 @@ type UndelegateMsg struct { // TxReceipt represents a transaction receipt that will serialize to the RPC representation. type TxReceipt struct { - BlockHash common.Hash `json:"blockHash"` - TransactionHash common.Hash `json:"transactionHash"` - BlockNumber uint64 `json:"blockNumber"` - TransactionIndex uint64 `json:"transactionIndex"` - GasUsed uint64 `json:"gasUsed"` - CumulativeGasUsed uint64 `json:"cumulativeGasUsed"` - ContractAddress *common.Address `json:"contractAddress"` - Logs []*types.Log `json:"logs"` - LogsBloom ethtypes.Bloom `json:"logsBloom"` - ShardID uint32 `json:"shardID"` - From string `json:"from"` - To string `json:"to"` - Root hexutil.Bytes `json:"root"` - Status uint `json:"status"` + BlockHash common.Hash `json:"blockHash"` + TransactionHash common.Hash `json:"transactionHash"` + BlockNumber uint64 `json:"blockNumber"` + TransactionIndex uint64 `json:"transactionIndex"` + GasUsed uint64 `json:"gasUsed"` + CumulativeGasUsed uint64 `json:"cumulativeGasUsed"` + ContractAddress common.Address `json:"contractAddress"` + Logs []*types.Log `json:"logs"` + LogsBloom ethtypes.Bloom `json:"logsBloom"` + ShardID uint32 `json:"shardID"` + From string `json:"from"` + To string `json:"to"` + Root hexutil.Bytes `json:"root"` + Status uint `json:"status"` } // StakingTxReceipt represents a staking transaction receipt that will serialize to the RPC representation. @@ -362,6 +362,7 @@ func NewTxReceipt( sender = senderAddr.String() receiver = "" } else { + // Handle response type for regular transaction sender, err = internal_common.AddressToBech32(senderAddr) if err != nil { return nil, err @@ -403,7 +404,7 @@ func NewTxReceipt( // If the ContractAddress is 20 0x0 bytes, assume it is not a contract creation if receipt.ContractAddress != (common.Address{}) { - txReceipt.ContractAddress = &receipt.ContractAddress + txReceipt.ContractAddress = receipt.ContractAddress } return txReceipt, nil } diff --git a/staking/types/delegation_test.go b/staking/types/delegation_test.go index feadda3cc0..ff0c25c02b 100644 --- a/staking/types/delegation_test.go +++ b/staking/types/delegation_test.go @@ -166,3 +166,123 @@ func TestNoEarlyUnlock(t *testing.T) { t.Errorf("should not allow early unlock") } } + +func TestMaxRateAtLess(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(27) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, true) + if result.Cmp(big.NewInt(0)) != 0 { + t.Errorf("should not allow unlock before 7") + } + finalLength := len(delegation.Undelegations) + if initialLength != finalLength { + t.Errorf("should not remove undelegations before 7") + } +} + +func TestMaxRateAtEqual(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(28) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, true) + if result.Cmp(big.NewInt(4000)) != 0 { + t.Errorf("should withdraw at 7") + } + finalLength := len(delegation.Undelegations) + if initialLength == finalLength { + t.Errorf("should remove undelegations at 7") + } +} + +func TestMaxRateAtExcess(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(29) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, true) + if result.Cmp(big.NewInt(0)) != 0 { + t.Errorf("should not withdraw at 8") + } + finalLength := len(delegation.Undelegations) + if initialLength == finalLength { + t.Errorf("should remove undelegations at 8") + } +} + +func TestNoMaxRateAtLess(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(27) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, false) + if result.Cmp(big.NewInt(0)) != 0 { + t.Errorf("should not allow unlock before 7") + } + finalLength := len(delegation.Undelegations) + if initialLength != finalLength { + t.Errorf("should not remove undelegations before 7") + } +} + +func TestNoMaxRateAtEqual(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(28) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, false) + if result.Cmp(big.NewInt(4000)) != 0 { + t.Errorf("should withdraw at 7") + } + finalLength := len(delegation.Undelegations) + if initialLength == finalLength { + t.Errorf("should remove undelegations at 7") + } +} + +func TestNoMaxRateAtExcess(t *testing.T) { + // recreate it so that all tests can run + delegation := NewDelegation(delegatorAddr, delegationAmt) + lastEpochInCommittee := big.NewInt(1) + curEpoch := big.NewInt(29) + epoch := big.NewInt(21) + amount := big.NewInt(4000) + delegation.Undelegate(epoch, amount) + initialLength := len(delegation.Undelegations) + + result := delegation.RemoveUnlockedUndelegations(curEpoch, lastEpochInCommittee, 7, true, false) + if result.Cmp(big.NewInt(4000)) != 0 { + t.Errorf("should withdraw at 8") + } + finalLength := len(delegation.Undelegations) + if initialLength == finalLength { + t.Errorf("should remove undelegations at 8") + } +}