Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api] correct signature's V value in returned web3 transaction #3952

Merged
merged 3 commits into from
Nov 1, 2023
Merged

[api] correct signature's V value in returned web3 transaction #3952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1ce239
[api] correct signature's V value in returned web3 transaction
dustinxie Oct 26, 2023
2ee9c17
fix lint
dustinxie Oct 26, 2023
94d4064
add test
dustinxie Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions action/rlp_tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func rlpRawHash(rawTx *types.Transaction, chainID uint32) (hash.Hash256, error)
}

func rlpSignedHash(tx *types.Transaction, chainID uint32, sig []byte) (hash.Hash256, error) {
signedTx, err := reconstructSignedRlpTxFromSig(tx, chainID, sig)
signedTx, err := RawTxToSignedTx(tx, chainID, sig)
if err != nil {
return hash.ZeroHash256, err
}
Expand All @@ -30,7 +30,8 @@ func rlpSignedHash(tx *types.Transaction, chainID uint32, sig []byte) (hash.Hash
return hash.BytesToHash256(h.Sum(nil)), nil
}

func reconstructSignedRlpTxFromSig(rawTx *types.Transaction, chainID uint32, sig []byte) (*types.Transaction, error) {
// RawTxToSignedTx converts the raw tx to corresponding signed tx
func RawTxToSignedTx(rawTx *types.Transaction, chainID uint32, sig []byte) (*types.Transaction, error) {
if len(sig) != 65 {
return nil, errors.Errorf("invalid signature length = %d, expecting 65", len(sig))
}
Expand All @@ -40,6 +41,8 @@ func reconstructSignedRlpTxFromSig(rawTx *types.Transaction, chainID uint32, sig
sc[64] -= 27
}

// TODO: currently all our web3 tx are EIP-155 protected tx
// in the future release, use proper signer for other supported tx types (EIP-1559, EIP-2930)
signedTx, err := rawTx.WithSignature(types.NewEIP155Signer(big.NewInt(int64(chainID))), sc)
if err != nil {
return nil, err
Expand Down Expand Up @@ -68,12 +71,12 @@ func DecodeRawTx(rawData string, chainID uint32) (tx *types.Transaction, sig []b
// extract signature and recover pubkey
v, r, s := tx.RawSignatureValues()
recID := uint32(v.Int64()) - 2*chainID - 8
sig = make([]byte, 64, 65)
sig = make([]byte, 65)
rSize := len(r.Bytes())
copy(sig[32-rSize:32], r.Bytes())
sSize := len(s.Bytes())
copy(sig[64-sSize:], s.Bytes())
sig = append(sig, byte(recID))
sig[64] = byte(recID)

// recover public key
rawHash := types.NewEIP155Signer(big.NewInt(int64(chainID))).Hash(tx)
Expand Down
68 changes: 46 additions & 22 deletions action/rlp_tx_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,8 @@ import (
"math/big"
"testing"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/rlp"
"github.com/iotexproject/go-pkgs/crypto"
"github.com/iotexproject/go-pkgs/hash"
"github.com/iotexproject/iotex-address/address"
"github.com/iotexproject/iotex-proto/golang/iotextypes"
Expand Down Expand Up @@ -289,26 +288,8 @@ func TestRlpDecodeVerify(t *testing.T) {
}

for _, v := range rlpTests {
encoded, err := hex.DecodeString(v.raw)
require.NoError(err)

// decode received RLP tx
tx := types.Transaction{}
require.NoError(rlp.DecodeBytes(encoded, &tx))

// extract signature and recover pubkey
w, r, s := tx.RawSignatureValues()
recID := uint32(w.Int64()) - 2*_evmNetworkID - 8
sig := make([]byte, 64, 65)
rSize := len(r.Bytes())
copy(sig[32-rSize:32], r.Bytes())
sSize := len(s.Bytes())
copy(sig[64-sSize:], s.Bytes())
sig = append(sig, byte(recID))

// recover public key
rawHash := types.NewEIP155Signer(big.NewInt(int64(_evmNetworkID))).Hash(&tx)
pubkey, err := crypto.RecoverPubkey(rawHash[:], sig)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate code, same as DecodeRawTx

tx, sig, pubkey, err := DecodeRawTx(v.raw, _evmNetworkID)
require.NoError(err)
require.Equal(v.pubkey, pubkey.HexString())
require.Equal(v.pkhash, hex.EncodeToString(pubkey.Hash()))
Expand All @@ -317,7 +298,7 @@ func TestRlpDecodeVerify(t *testing.T) {
pb := &iotextypes.Action{
Encoding: iotextypes.Encoding_ETHEREUM_RLP,
}
pb.Core = convertToNativeProto(&tx, v.actType)
pb.Core = convertToNativeProto(tx, v.actType)
pb.SenderPubKey = pubkey.Bytes()
pb.Signature = sig

Expand Down Expand Up @@ -352,6 +333,7 @@ func TestRlpDecodeVerify(t *testing.T) {
require.True(bytes.Equal(sig, selp.signature))
raw, err := selp.envelopeHash()
require.NoError(err)
rawHash := types.NewEIP155Signer(big.NewInt(int64(_evmNetworkID))).Hash(tx)
require.True(bytes.Equal(rawHash[:], raw[:]))
require.NotEqual(raw, h)
require.NoError(selp.VerifySignature())
Expand Down Expand Up @@ -379,3 +361,45 @@ func convertToNativeProto(tx *types.Transaction, actType string) *iotextypes.Act
panic("unsupported")
}
}

func TestIssue3944(t *testing.T) {
r := require.New(t)
// the sample tx below is the web3 format tx on the testnet:
// https://testnet.iotexscan.io/tx/fcaf377ff3cc785d60c58de7e121d6a2e79e1c58c189ea8641f3ea61f7605285
// or you can get it using ethclient code:
// {
// cli, _ := ethclient.Dial("https://babel-api.testnet.iotex.io")
// tx, _, err := cli.TransactionByHash(context.Background(), common.HexToHash("0xfcaf377ff3cc785d60c58de7e121d6a2e79e1c58c189ea8641f3ea61f7605285"))
// }
var (
hash = "0xfcaf377ff3cc785d60c58de7e121d6a2e79e1c58c189ea8641f3ea61f7605285"
data, _ = hex.DecodeString("f3fef3a3000000000000000000000000fdff3eafde9a0cc42d18aab2a7454b1105f19edf00000000000000000000000000000000000000000000002086ac351052600000")
to = common.HexToAddress("0xd313b3131e238C635f2fE4a84EaDaD71b3ed25fa")
sig, _ = hex.DecodeString("adff1da88c93f4e80c27bab0d613147fb7aeeed6e976231695de52cd9ac5aa8a3094e02759b838514f8376e05ceb266badc791ac2e7045ee7c15e58fc626980b1b")
)
tx := types.NewTx(&types.LegacyTx{
Nonce: 83,
To: &to,
Value: new(big.Int),
Gas: 39295,
GasPrice: big.NewInt(1000000000000),
Data: data,
V: big.NewInt(int64(sig[64])),
R: new(big.Int).SetBytes(sig[:32]),
S: new(big.Int).SetBytes(sig[32:64]),
})

v, q, s := tx.RawSignatureValues()
r.Equal(sig[:32], q.Bytes())
r.Equal(sig[32:64], s.Bytes())
r.Equal("1b", v.Text(16))
r.NotEqual(hash, tx.Hash().Hex()) // hash does not match with wrong V value in signature

tx1, err := RawTxToSignedTx(tx, 4690, sig)
r.NoError(err)
v, q, s = tx1.RawSignatureValues()
r.Equal(sig[:32], q.Bytes())
r.Equal(sig[32:64], s.Bytes())
r.Equal("9415", v.String()) // this is the correct V value corresponding to chainID = 4690
r.Equal(hash, tx1.Hash().Hex())
}
15 changes: 4 additions & 11 deletions api/web3server_marshal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package api
import (
"encoding/hex"
"encoding/json"
"math/big"

"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/core/types"
Expand Down Expand Up @@ -57,7 +56,6 @@ type (
ethTx *types.Transaction
receipt *action.Receipt
pubkey crypto.PublicKey
signature []byte
}

getReceiptResult struct {
Expand Down Expand Up @@ -229,12 +227,7 @@ func (obj *getTransactionResult) MarshalJSON() ([]byte, error) {
}
value, _ := intStrToHex(obj.ethTx.Value().String())
gasPrice, _ := intStrToHex(obj.ethTx.GasPrice().String())

// TODO: if transaction is support EIP-155, we need to add chainID to v
vVal := uint64(obj.signature[64])
if vVal < 27 {
vVal += 27
}
v, r, s := obj.ethTx.RawSignatureValues()
envestcc marked this conversation as resolved.
Show resolved Hide resolved

return json.Marshal(&struct {
Hash string `json:"hash"`
Expand Down Expand Up @@ -263,9 +256,9 @@ func (obj *getTransactionResult) MarshalJSON() ([]byte, error) {
GasPrice: gasPrice,
Gas: uint64ToHex(obj.ethTx.Gas()),
Input: byteToHex(obj.ethTx.Data()),
R: hexutil.EncodeBig(new(big.Int).SetBytes(obj.signature[:32])),
S: hexutil.EncodeBig(new(big.Int).SetBytes(obj.signature[32:64])),
V: uint64ToHex(vVal),
R: hexutil.EncodeBig(r),
S: hexutil.EncodeBig(s),
V: hexutil.EncodeBig(v),
})
}

Expand Down
23 changes: 14 additions & 9 deletions api/web3server_marshal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,13 +154,15 @@ func TestBlockObjectMarshal(t *testing.T) {
})

t.Run("BlockWithDetail", func(t *testing.T) {
raw := types.NewContractCreation(2, unit.ConvertIotxToRau(1000), 21000, unit.ConvertIotxToRau(1), []byte{})
ethTx, err := action.RawTxToSignedTx(raw, 0, sevlp.Signature())
require.NoError(err)
tx := &getTransactionResult{
blockHash: _testBlkHash,
to: nil,
ethTx: types.NewContractCreation(2, unit.ConvertIotxToRau(1000), 21000, unit.ConvertIotxToRau(1), []byte{}),
ethTx: ethTx,
receipt: blk.Receipts[0],
pubkey: sevlp.SrcPubkey(),
signature: sevlp.Signature(),
}
res, err := json.Marshal(&getBlockResult{
blk: &blk,
Expand Down Expand Up @@ -226,14 +228,15 @@ func TestTransactionObjectMarshal(t *testing.T) {
}

t.Run("ContractCreation", func(t *testing.T) {
raw := types.NewContractCreation(1, big.NewInt(10), 21000, big.NewInt(0), []byte{})
sig, _ := hex.DecodeString("363964383961306166323764636161363766316236326133383335393464393735393961616464326237623136346362343131326161386464666434326638391b")
tx, err := action.RawTxToSignedTx(raw, 4690, sig)
res, err := json.Marshal(&getTransactionResult{
blockHash: _testBlkHash,
to: nil,
ethTx: types.NewContractCreation(1, big.NewInt(10), 21000, big.NewInt(0), []byte{}),
ethTx: tx,
receipt: receipt,
pubkey: _testPubKey,
// TODO: should decode signature from hex string
signature: []byte("69d89a0af27dcaa67f1b62a383594d97599aadd2b7b164cb4112aa8ddfd42f895649075cae1b7216c43a491c5e9be68d1d9a27b863d71155ecdd7c95dab5394f01"),
})
require.NoError(err)
require.JSONEq(`
Expand All @@ -251,7 +254,7 @@ func TestTransactionObjectMarshal(t *testing.T) {
"input":"0x",
"r":"0x3639643839613061663237646361613637663162363261333833353934643937",
"s":"0x3539396161646432623762313634636234313132616138646466643432663839",
"v":"0x35"
"v":"0x24c7"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the correct value corresponding to chainID = 4690

}
`, string(res))
})
Expand All @@ -262,10 +265,13 @@ func TestTransactionObjectMarshal(t *testing.T) {
data, _ := hex.DecodeString("1fad948c0000000000000000000000000000000000000000000000000000000000000040000000000000000000000000ec4daee51c4bf81bd00af165f8ca66823ee3b12a000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000003e24491a4f2a946e30baf624fda6b4484d106c12000000000000000000000000000000000000000000000000000000000000005b000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000001800000000000000000000000000000000000000000000000000000000000009fd90000000000000000000000000000000000000000000000000000000000011da4000000000000000000000000000000000000000000000000000000000000db26000000000000000000000000000000000000000000000000000000e8d4a51000000000000000000000000000000000000000000000000000000000e8d4a510000000000000000000000000000000000000000000000000000000000000000240000000000000000000000000000000000000000000000000000000000000030000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000084b61d27f6000000000000000000000000065e1164818487818e6ba714e8d80b91718ad75800000000000000000000000000000000000000000000000000038d7ea4c6800000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000095ccc7012efb2e65aa31752f3ac01e23817c08a47500000000000000000000000000000000000000000000000000000000650af9f8000000000000000000000000000000000000000000000000000000006509a878b5acba7277159ae6fa661ed1988cc10ac2c96c58dc332bde2a6dc0d8531ea3924d9d04cda681c271411250ae7d9e9aea47661dba67a66f08d19804a255e45c561b0000000000000000000000000000000000000000000000000000000000000000000000000000000000004160daa88165299ca7e585d5d286cee98b54397b57ac704b74331a48d67651195322ef3884c7d60023333f2542a07936f34edc9efa3cbd19e8cd0f8972c54171a21b00000000000000000000000000000000000000000000000000000000000000")
pubkey, _ := crypto.HexStringToPublicKey("04806b217cb0b6a675974689fd99549e525d967287eee9a62dc4e598eea981b8158acfe026da7bf58397108abd0607672832c28ef3bc7b5855077f6e67ab5fc096")
actHash, _ := hash.HexStringToHash256("cbc2560d986d79a46bfd96a08d18c6045b29f97352c1360289e371d9cffd6b6a")
raw := types.NewContractCreation(305, big.NewInt(0), 297131, big.NewInt(1000000000000), data)
tx, err := action.RawTxToSignedTx(raw, 0, sig)
require.NoError(err)
res, err := json.Marshal(&getTransactionResult{
blockHash: blkHash,
to: &contract,
ethTx: types.NewContractCreation(305, big.NewInt(0), 297131, big.NewInt(1000000000000), data),
ethTx: tx,
receipt: &action.Receipt{
Status: 1,
BlockHeight: 22354907,
Expand All @@ -274,8 +280,7 @@ func TestTransactionObjectMarshal(t *testing.T) {
ContractAddress: "",
TxIndex: 0,
},
pubkey: pubkey,
signature: sig,
pubkey: pubkey,
})
require.NoError(err)
require.JSONEq(`
Expand Down
5 changes: 5 additions & 0 deletions api/web3server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ func TestGetBlockByNumber(t *testing.T) {
Block: &blk,
Receipts: receipts,
}, nil)
core.EXPECT().ChainID().Return(uint32(0))

in := gjson.Parse(`{"params":["1", true]}`)
ret, err := web3svr.getBlockByNumber(&in)
Expand Down Expand Up @@ -504,6 +505,7 @@ func TestGetBlockByHash(t *testing.T) {
Block: &blk,
Receipts: receipts,
}, nil)
core.EXPECT().ChainID().Return(uint32(0))

blkHash := blk.HashBlock()
in := gjson.Parse(fmt.Sprintf(`{"params":["0x%s", true]}`, hex.EncodeToString(blkHash[:])))
Expand Down Expand Up @@ -540,6 +542,7 @@ func TestGetTransactionByHash(t *testing.T) {
}
core.EXPECT().ActionByActionHash(gomock.Any()).Return(selp, hash.Hash256b([]byte("test")), uint64(0), uint32(0), nil)
core.EXPECT().ReceiptByActionHash(gomock.Any()).Return(receipt, nil)
core.EXPECT().ChainID().Return(uint32(0))

in := gjson.Parse(fmt.Sprintf(`{"params":["0x%s", true]}`, hex.EncodeToString(txHash[:])))
ret, err := web3svr.getTransactionByHash(&in)
Expand Down Expand Up @@ -695,6 +698,7 @@ func TestGetTransactionByBlockHashAndIndex(t *testing.T) {
Block: &blk,
Receipts: receipts,
}, nil)
core.EXPECT().ChainID().Return(uint32(0))

blkHash := blk.HashBlock()
in := gjson.Parse(fmt.Sprintf(`{"params":["0x%s", "0"]}`, hex.EncodeToString(blkHash[:])))
Expand Down Expand Up @@ -733,6 +737,7 @@ func TestGetTransactionByBlockNumberAndIndex(t *testing.T) {
Block: &blk,
Receipts: receipts,
}, nil)
core.EXPECT().ChainID().Return(uint32(0))

in := gjson.Parse(`{"params":["0x1", "0"]}`)
ret, err := web3svr.getTransactionByBlockNumberAndIndex(&in)
Expand Down
7 changes: 5 additions & 2 deletions api/web3server_utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,16 @@ func (svr *web3Handler) getTransactionFromActionInfo(blkHash hash.Hash256, selp
if err != nil {
return nil, err
}
tx, err := action.RawTxToSignedTx(ethTx, svr.coreService.ChainID(), selp.Signature())
Liuhaai marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
return &getTransactionResult{
blockHash: blkHash,
to: to,
ethTx: ethTx,
ethTx: tx,
receipt: receipt,
pubkey: selp.SrcPubkey(),
signature: selp.Signature(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why could signature be deleted?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not needed anymore, now the signed tx is generated, which contains correct signature value

}, nil
}

Expand Down