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.

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

Fixes for Trezor tx signing errors after the recent firmware upgrade #162

Merged
merged 5 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions common/heights.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ const HeightEnableTheta3 uint64 = 10968061 // approximate time: 12pm June 30, 20
// HeightRPCCompatibility specifies the block height to enable Ethereum compatible RPC support
const HeightRPCCompatibility uint64 = 11354820 // approximate time: 12pm July 30, 2021 PT

// HeightTxWrapperExtension specifies the block height to extend the Tx Wrapper
const HeightTxWrapperExtension uint64 = 1000000000

// CheckpointInterval defines the interval between checkpoints.
const CheckpointInterval = int64(100)

Expand Down
2 changes: 1 addition & 1 deletion consensus/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ func (e *ConsensusEngine) shouldProposeByID(previousBlock common.Hash, epoch uin
if proposer.ID().Hex() != id {
e.logger.WithFields(log.Fields{
"expectedProposer": proposer.ID().Hex(),
"tip": previousBlock,
"tip": previousBlock.Hex(),
"epoch": epoch,
}).Debug("shouldProposeByID=false")

Expand Down
14 changes: 10 additions & 4 deletions ledger/execution/executils.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,14 +162,14 @@ func validateInputsBasic(ins []types.TxInput) result.Result {
}

// Validate inputs and compute total amount of coins
func validateInputsAdvanced(accounts map[string]*types.Account, signBytes []byte, ins []types.TxInput) (total types.Coins, res result.Result) {
func validateInputsAdvanced(accounts map[string]*types.Account, signBytes []byte, ins []types.TxInput, blockHeight uint64) (total types.Coins, res result.Result) {
total = types.NewCoins(0, 0)
for _, in := range ins {
acc := accounts[string(in.Address[:])]
if acc == nil {
panic("validateInputsAdvanced() expects account in accounts")
}
res = validateInputAdvanced(acc, signBytes, in)
res = validateInputAdvanced(acc, signBytes, in, blockHeight)
if res.IsError() {
return
}
Expand All @@ -179,7 +179,7 @@ func validateInputsAdvanced(accounts map[string]*types.Account, signBytes []byte
return total, result.OK
}

func validateInputAdvanced(acc *types.Account, signBytes []byte, in types.TxInput) result.Result {
func validateInputAdvanced(acc *types.Account, signBytes []byte, in types.TxInput, blockHeight uint64) result.Result {
// Check sequence/coins
seq, balance := acc.Sequence, acc.Balance
if seq+1 != in.Sequence {
Expand All @@ -194,7 +194,13 @@ func validateInputAdvanced(acc *types.Account, signBytes []byte, in types.TxInpu
}

// Check signatures
if !in.Signature.Verify(signBytes, acc.Address) {
signatureValid := in.Signature.Verify(signBytes, acc.Address)
if blockHeight >= common.HeightTxWrapperExtension {
signBytesV2 := types.ChangeEthereumTxWrapper(signBytes, 2)
signatureValid = signatureValid || in.Signature.Verify(signBytesV2, acc.Address)
}

if !signatureValid {
return result.Error("Signature verification failed, SignBytes: %v",
hex.EncodeToString(signBytes)).WithErrorCode(result.CodeInvalidSignature)
}
Expand Down
12 changes: 6 additions & 6 deletions ledger/execution/regular_tx_execution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,12 @@ func TestValidateInputsAdvanced(t *testing.T) {
signBytes := tx.SignBytes(et.chainID)

//test bad case, unsigned
totalCoins, res := validateInputsAdvanced(accMap, signBytes, tx.Inputs)
totalCoins, res := validateInputsAdvanced(accMap, signBytes, tx.Inputs, 1)
assert.True(res.IsError(), "validateInputsAdvanced: expected an error on an unsigned tx input")

//test good case sgined
et.signSendTx(tx, accIn1, accIn2, accIn3, et.accOut)
totalCoins, res = validateInputsAdvanced(accMap, signBytes, tx.Inputs)
totalCoins, res = validateInputsAdvanced(accMap, signBytes, tx.Inputs, 1)
assert.True(res.IsOK(), "validateInputsAdvanced: expected no error on good tx input. Error: %v", res.Message)

txTotalCoins := tx.Inputs[0].Coins.
Expand All @@ -161,25 +161,25 @@ func TestValidateInputAdvanced(t *testing.T) {
signBytes := tx.SignBytes(et.chainID)

//unsigned case
res := validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0])
res := validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0], 1)
assert.True(res.IsError(), "validateInputAdvanced: expected error on tx input without signature")

//good signed case
et.signSendTx(tx, et.accIn, et.accOut)
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0])
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0], 1)
assert.True(res.IsOK(), "validateInputAdvanced: expected no error on good tx input. Error: %v", res.Message)

//bad sequence case
et.accIn.Sequence = 1
et.signSendTx(tx, et.accIn, et.accOut)
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0])
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0], 1)
assert.Equal(result.CodeInvalidSequence, res.Code, "validateInputAdvanced: expected error on tx input with bad sequence")
et.accIn.Sequence = 0 //restore sequence

//bad balance case
et.accIn.Balance = types.NewCoins(2, 0)
et.signSendTx(tx, et.accIn, et.accOut)
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0])
res = validateInputAdvanced(&et.accIn.Account, signBytes, tx.Inputs[0], 1)
assert.Equal(result.CodeInsufficientFund, res.Code,
"validateInputAdvanced: expected error on tx input with insufficient funds %v", et.accIn.Sequence)
}
Expand Down
2 changes: 1 addition & 1 deletion ledger/execution/tx_deposit_stake.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (exec *DepositStakeExecutor) sanityCheck(chainID string, view *st.StoreView
}

signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source, blockHeight)
if res.IsError() {
logger.Debugf(fmt.Sprintf("validateSourceAdvanced failed on %v: %v", tx.Source.Address.Hex(), res))
return res
Expand Down
4 changes: 2 additions & 2 deletions ledger/execution/tx_release_fund.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func NewReleaseFundTxExecutor(state *st.LedgerState) *ReleaseFundTxExecutor {
}

func (exec *ReleaseFundTxExecutor) sanityCheck(chainID string, view *st.StoreView, transaction types.Tx) result.Result {
blockHeight := view.Height() + 1 // the view points to the parent of the current block
tx := transaction.(*types.ReleaseFundTx)

// Validate source, basic
Expand All @@ -44,13 +45,12 @@ func (exec *ReleaseFundTxExecutor) sanityCheck(chainID string, view *st.StoreVie

// Validate input, advanced
signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source, blockHeight)
if res.IsError() {
logger.Debugf(fmt.Sprintf("validateSourceAdvanced failed on %v: %v", tx.Source.Address.Hex(), res))
return res
}

blockHeight := view.Height() + 1 // the view points to the parent of the current block
if minTxFee, success := sanityCheckForFee(tx.Fee, blockHeight); !success {
return result.Error("Insufficient fee. Transaction fee needs to be at least %v TFuelWei",
minTxFee).WithErrorCode(result.CodeInvalidFee)
Expand Down
4 changes: 2 additions & 2 deletions ledger/execution/tx_reserve_fund.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func NewReserveFundTxExecutor(state *st.LedgerState) *ReserveFundTxExecutor {
}

func (exec *ReserveFundTxExecutor) sanityCheck(chainID string, view *st.StoreView, transaction types.Tx) result.Result {
blockHeight := view.Height() + 1 // the view points to the parent of the current block
tx := transaction.(*types.ReserveFundTx)

// Validate source, basic
Expand All @@ -44,7 +45,7 @@ func (exec *ReserveFundTxExecutor) sanityCheck(chainID string, view *st.StoreVie

// Validate input, advanced
signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source, blockHeight)
if res.IsError() {
logger.Debugf(fmt.Sprintf("validateSourceAdvanced failed on %v: %v", tx.Source.Address.Hex(), res))
return res
Expand All @@ -62,7 +63,6 @@ func (exec *ReserveFundTxExecutor) sanityCheck(chainID string, view *st.StoreVie
WithErrorCode(result.CodeInvalidFundToReserve)
}

blockHeight := view.Height() + 1 // the view points to the parent of the current block
if minTxFee, success := sanityCheckForFee(tx.Fee, blockHeight); !success {
return result.Error("Insufficient fee. Transaction fee needs to be at least %v TFuelWei",
minTxFee).WithErrorCode(result.CodeInvalidFee)
Expand Down
2 changes: 1 addition & 1 deletion ledger/execution/tx_send.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (exec *SendTxExecutor) sanityCheck(chainID string, view *st.StoreView, tran

// Validate inputs and outputs, advanced
signBytes := tx.SignBytes(chainID)
inTotal, res := validateInputsAdvanced(accounts, signBytes, tx.Inputs)
inTotal, res := validateInputsAdvanced(accounts, signBytes, tx.Inputs, blockHeight)
if res.IsError() {
return res
}
Expand Down
8 changes: 7 additions & 1 deletion ledger/execution/tx_smart_contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ func (exec *SmartContractTxExecutor) sanityCheck(chainID string, view *st.StoreV

// Check signatures
signBytes := tx.SignBytes(chainID)
if !tx.From.Signature.Verify(signBytes, tx.From.Address) {
nativeSignatureValid := tx.From.Signature.Verify(signBytes, tx.From.Address)
if blockHeight >= common.HeightTxWrapperExtension {
signBytesV2 := types.ChangeEthereumTxWrapper(signBytes, 2)
nativeSignatureValid = nativeSignatureValid || tx.From.Signature.Verify(signBytesV2, tx.From.Address)
}

if !nativeSignatureValid {
if blockHeight < common.HeightRPCCompatibility {
return result.Error("Signature verification failed, SignBytes: %v",
hex.EncodeToString(signBytes)).WithErrorCode(result.CodeInvalidSignature)
Expand Down
4 changes: 2 additions & 2 deletions ledger/execution/tx_split_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ func NewSplitRuleTxExecutor(state *st.LedgerState) *SplitRuleTxExecutor {
}

func (exec *SplitRuleTxExecutor) sanityCheck(chainID string, view *st.StoreView, transaction types.Tx) result.Result {
blockHeight := view.Height() + 1 // the view points to the parent of the current block
tx := transaction.(*types.SplitRuleTx)

res := tx.Initiator.ValidateBasic()
Expand All @@ -43,12 +44,11 @@ func (exec *SplitRuleTxExecutor) sanityCheck(chainID string, view *st.StoreView,

// Validate inputs and outputs, advanced
signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(initiatorAccount, signBytes, tx.Initiator)
res = validateInputAdvanced(initiatorAccount, signBytes, tx.Initiator, blockHeight)
if res.IsError() {
return res
}

blockHeight := view.Height() + 1 // the view points to the parent of the current block
if minTxFee, success := sanityCheckForFee(tx.Fee, blockHeight); !success {
return result.Error("Insufficient fee. Transaction fee needs to be at least %v TFuelWei",
minTxFee).WithErrorCode(result.CodeInvalidFee)
Expand Down
2 changes: 1 addition & 1 deletion ledger/execution/tx_stake_reward_distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (exec *StakeRewardDistributionTxExecutor) sanityCheck(chainID string, view

// Validate inputs and outputs, advanced
signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(stakeHolderAccount, signBytes, tx.Holder)
res = validateInputAdvanced(stakeHolderAccount, signBytes, tx.Holder, blockHeight)
if res.IsError() {
return res
}
Expand Down
4 changes: 2 additions & 2 deletions ledger/execution/tx_withdraw_stake.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ func NewWithdrawStakeExecutor(state *st.LedgerState) *WithdrawStakeExecutor {
}

func (exec *WithdrawStakeExecutor) sanityCheck(chainID string, view *st.StoreView, transaction types.Tx) result.Result {
blockHeight := view.Height() + 1 // the view points to the parent of the current block
tx := transaction.(*types.WithdrawStakeTx)

res := tx.Source.ValidateBasic()
Expand All @@ -42,13 +43,12 @@ func (exec *WithdrawStakeExecutor) sanityCheck(chainID string, view *st.StoreVie
}

signBytes := tx.SignBytes(chainID)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source)
res = validateInputAdvanced(sourceAccount, signBytes, tx.Source, blockHeight)
if res.IsError() {
logger.Debugf(fmt.Sprintf("validateSourceAdvanced failed on %v: %v", tx.Source.Address.Hex(), res))
return res
}

blockHeight := view.Height() + 1 // the view points to the parent of the current block
if minTxFee, success := sanityCheckForFee(tx.Fee, blockHeight); !success {
return result.Error("Insufficient fee. Transaction fee needs to be at least %v TFuelWei",
minTxFee).WithErrorCode(result.CodeInvalidFee)
Expand Down
42 changes: 42 additions & 0 deletions ledger/types/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,48 @@ func addPrefixForSignBytes(signBytes common.Bytes) common.Bytes {
return signBytes
}

type EthereumTxWrapperV2 struct {
AccountNonce uint64 `json:"nonce" gencodec:"required"`
Price *big.Int `json:"gasPrice" gencodec:"required"`
GasLimit uint64 `json:"gas" gencodec:"required"`
Recipient *common.Address `json:"to" rlp:"nil"` // nil means contract creation
Amount *big.Int `json:"value" gencodec:"required"`
Payload []byte `json:"input" gencodec:"required"`
ChainID uint64 `json:"chainId" gencodec:"required"`
EIP155Field1 uint
EIP155Field2 uint
}

func ChangeEthereumTxWrapper(origSignBytes common.Bytes, wrapperVersion uint) common.Bytes {
wrappedTx := &EthereumTxWrapper{}
err := rlp.DecodeBytes(origSignBytes, wrappedTx)
if err != nil {
log.Panic(err)
}

if wrapperVersion == 2 {
wrappedTx := EthereumTxWrapperV2{
AccountNonce: wrappedTx.AccountNonce,
Price: wrappedTx.Price,
GasLimit: wrappedTx.GasLimit,
Recipient: wrappedTx.Recipient,
Amount: wrappedTx.Amount,
Payload: wrappedTx.Payload,
ChainID: uint64(1),
EIP155Field1: uint(0),
EIP155Field2: uint(0),
}
signBytes, err := rlp.EncodeToBytes(wrappedTx)
if err != nil {
log.Panic(err)
}
return signBytes
}

log.Panic(fmt.Errorf("invalid ethereum tx wrapper version"))
return common.Bytes{}
}

// For replay attack protection
// https://chainid.network/
const CHAIN_ID_OFFSET int64 = 360
Expand Down
2 changes: 1 addition & 1 deletion version/version_number.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.1.1
3.1.2