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

feat: Revert sig_block_height #434

Merged
merged 8 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 7 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
1,181 changes: 148 additions & 1,033 deletions CHANGELOG.md

Large diffs are not rendered by default.

4 changes: 0 additions & 4 deletions baseapp/accountwgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,6 @@ func (tx AccountLockTestTx) ValidateBasic() error {
return nil
}

func (tx AccountLockTestTx) GetSigBlockHeight() uint64 {
return 0
}

func newTestPrivKeys(num int) []*secp256k1.PrivKey {
privs := make([]*secp256k1.PrivKey, 0, num)
for i := 0; i < num; i++ {
Expand Down
5 changes: 2 additions & 3 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,9 +696,8 @@ func (tx *txTest) setFailOnHandler(fail bool) {
}

// Implements Tx
func (tx txTest) GetMsgs() []sdk.Msg { return tx.Msgs }
func (tx txTest) ValidateBasic() error { return nil }
func (tx txTest) GetSigBlockHeight() uint64 { return 0 }
func (tx txTest) GetMsgs() []sdk.Msg { return tx.Msgs }
func (tx txTest) ValidateBasic() error { return nil }

const (
routeMsgCounter = "msgCounter"
Expand Down
5 changes: 3 additions & 2 deletions baseapp/msg_service_router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,9 @@ func TestMsgService(t *testing.T) {

// Second round: all signer infos are set, so each signer can sign.
signerData := authsigning.SignerData{
ChainID: "test",
Sequence: 0,
ChainID: "test",
AccountNumber: 0,
Sequence: 0,
}
sigV2, err = tx.SignWithPrivKey(
encCfg.TxConfig.SignModeHandler().DefaultMode(), signerData,
Expand Down
4 changes: 2 additions & 2 deletions client/account_retriever.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
type Account interface {
GetAddress() sdk.AccAddress
GetPubKey() cryptotypes.PubKey // can return nil.
GetAccountNumber() uint64
GetSequence() uint64
}

Expand All @@ -17,8 +18,7 @@ type Account interface {
// for signing.
type AccountRetriever interface {
GetAccount(clientCtx Context, addr sdk.AccAddress) (Account, error)
GetLatestHeight(clientCtx Context) (uint64, error)
GetAccountWithHeight(clientCtx Context, addr sdk.AccAddress) (Account, int64, error)
EnsureExists(clientCtx Context, addr sdk.AccAddress) error
GetAccountSequence(clientCtx Context, addr sdk.AccAddress) (accSeq uint64, err error)
GetAccountNumberSequence(clientCtx Context, addr sdk.AccAddress) (accNum uint64, accSeq uint64, err error)
}
18 changes: 0 additions & 18 deletions client/docs/swagger-ui/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4926,9 +4926,6 @@ paths:
sig_verify_cost_secp256k1:
type: string
format: uint64
valid_sig_block_period:
type: string
format: uint64
description: >-
QueryParamsResponse is the response type for the Query/Params RPC
method.
Expand Down Expand Up @@ -35180,9 +35177,6 @@ definitions:
sig_verify_cost_secp256k1:
type: string
format: uint64
valid_sig_block_period:
type: string
format: uint64
description: Params defines the parameters for the auth module.
lbm.auth.v1.QueryAccountResponse:
type: object
Expand Down Expand Up @@ -35274,9 +35268,6 @@ definitions:
sig_verify_cost_secp256k1:
type: string
format: uint64
valid_sig_block_period:
type: string
format: uint64
description: QueryParamsResponse is the response type for the Query/Params RPC method.
lbm.bank.v1.DenomUnit:
type: object
Expand Down Expand Up @@ -47143,15 +47134,6 @@ definitions:
appropriate fee grant does not exist or the chain does

not support fee grants, this will fail
sig_block_height:
type: string
format: uint64
description: >-
sig block height is available between current block height and current
block height - `VALID_SIG_BLOCK_PERIOD`

this is used for distinguish signatures instead of account number.
this is mandatory.
description: |-
AuthInfo describes the fee and signer modes that are used to sign a
transaction.
Expand Down
4 changes: 2 additions & 2 deletions client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const (
FlagGasAdjustment = "gas-adjustment"
FlagFrom = "from"
FlagName = "name"
FlagSigBlockHeight = "sig-block-height"
FlagAccountNumber = "account-number"
FlagSequence = "sequence"
FlagMemo = "memo"
FlagFees = "fees"
Expand Down Expand Up @@ -101,7 +101,7 @@ func AddTxFlagsToCmd(cmd *cobra.Command) {
cmd.Flags().StringP(ostcli.OutputFlag, "o", "json", "Output format (text|json)")
cmd.Flags().String(FlagKeyringDir, "", "The client Keyring directory; if omitted, the default 'home' directory will be used")
cmd.Flags().String(FlagFrom, "", "Name or address of private key with which to sign")
cmd.Flags().Uint64P(FlagSigBlockHeight, "n", 0, "The block height to be included in the tx body to protect from replaying")
cmd.Flags().Uint64P(FlagAccountNumber, "a", 0, "The account number of the signing account (offline mode only)")
cmd.Flags().Uint64P(FlagSequence, "s", 0, "The sequence number of the signing account (offline mode only)")
cmd.Flags().String(FlagMemo, "", "Memo to send along with transaction")
cmd.Flags().String(FlagFees, "", "Fees to pay along with transaction; eg: 10uatom")
Expand Down
18 changes: 10 additions & 8 deletions client/test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ var (
// TestAccount represents a client Account that can be used in unit tests
type TestAccount struct {
Address sdk.AccAddress
Num uint64
Seq uint64
}

Expand All @@ -28,6 +29,11 @@ func (t TestAccount) GetPubKey() cryptotypes.PubKey {
return nil
}

// GetAccountNumber implements client Account.GetAccountNumber
func (t TestAccount) GetAccountNumber() uint64 {
return t.Num
}

// GetSequence implements client Account.GetSequence
func (t TestAccount) GetSequence() uint64 {
return t.Seq
Expand All @@ -48,10 +54,6 @@ func (t TestAccountRetriever) GetAccount(_ Context, addr sdk.AccAddress) (Accoun
return acc, nil
}

func (t TestAccountRetriever) GetLatestHeight(_ Context) (uint64, error) {
return 0, nil
}

// GetAccountWithHeight implements AccountRetriever.GetAccountWithHeight
func (t TestAccountRetriever) GetAccountWithHeight(clientCtx Context, addr sdk.AccAddress) (Account, int64, error) {
acc, err := t.GetAccount(clientCtx, addr)
Expand All @@ -71,11 +73,11 @@ func (t TestAccountRetriever) EnsureExists(_ Context, addr sdk.AccAddress) error
return nil
}

// GetAccountSequence implements AccountRetriever.GetAccountSequence
func (t TestAccountRetriever) GetAccountSequence(_ Context, addr sdk.AccAddress) (accSeq uint64, err error) {
// GetAccountNumberSequence implements AccountRetriever.GetAccountNumberSequence
func (t TestAccountRetriever) GetAccountNumberSequence(_ Context, addr sdk.AccAddress) (accNum uint64, accSeq uint64, err error) {
acc, ok := t.Accounts[addr.String()]
if !ok {
return 0, fmt.Errorf("account %s not found", addr)
return 0, 0, fmt.Errorf("account %s not found", addr)
}
return acc.Seq, nil
return acc.Num, acc.Seq, nil
}
14 changes: 7 additions & 7 deletions client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type Factory struct {
keybase keyring.Keyring
txConfig client.TxConfig
accountRetriever client.AccountRetriever
sigBlockHeight uint64
accountNumber uint64
sequence uint64
gas uint64
timeoutHeight uint64
Expand All @@ -41,7 +41,7 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
signMode = signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON
}

sigBlockHeight, _ := flagSet.GetUint64(flags.FlagSigBlockHeight)
accNum, _ := flagSet.GetUint64(flags.FlagAccountNumber)
accSeq, _ := flagSet.GetUint64(flags.FlagSequence)
gasAdj, _ := flagSet.GetFloat64(flags.FlagGasAdjustment)
memo, _ := flagSet.GetString(flags.FlagMemo)
Expand All @@ -57,7 +57,7 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
chainID: clientCtx.ChainID,
gas: gasSetting.Gas,
simulateAndExecute: gasSetting.Simulate,
sigBlockHeight: sigBlockHeight,
accountNumber: accNum,
sequence: accSeq,
timeoutHeight: timeoutHeight,
gasAdjustment: gasAdj,
Expand All @@ -74,7 +74,7 @@ func NewFactoryCLI(clientCtx client.Context, flagSet *pflag.FlagSet) Factory {
return f
}

func (f Factory) SigBlockHeight() uint64 { return f.sigBlockHeight }
func (f Factory) AccountNumber() uint64 { return f.accountNumber }
func (f Factory) Sequence() uint64 { return f.sequence }
func (f Factory) Gas() uint64 { return f.gas }
func (f Factory) GasAdjustment() float64 { return f.gasAdjustment }
Expand Down Expand Up @@ -154,9 +154,9 @@ func (f Factory) WithMemo(memo string) Factory {
return f
}

// WithSigBlockHeight returns a copy of the Factory with an updated sig block height.
func (f Factory) WithSigBlockHeight(sigBlockHeight uint64) Factory {
f.sigBlockHeight = sigBlockHeight
// WithAccountNumber returns a copy of the Factory with an updated account number.
func (f Factory) WithAccountNumber(accnum uint64) Factory {
f.accountNumber = accnum
return f
}

Expand Down
1 change: 0 additions & 1 deletion client/tx/legacy.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func CopyTx(tx signing.Tx, builder client.TxBuilder, ignoreSignatureError bool)
builder.SetMemo(tx.GetMemo())
builder.SetFeeAmount(tx.GetFee())
builder.SetGasLimit(tx.GetGas())
builder.SetSigBlockHeight(tx.GetSigBlockHeight())
builder.SetTimeoutHeight(tx.GetTimeoutHeight())

return nil
Expand Down
2 changes: 0 additions & 2 deletions client/tx/legacy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
const (
memo = "waboom"
gas = uint64(10000)
sbh = 1
timeoutHeight = 5
)

Expand All @@ -45,7 +44,6 @@ func buildTestTx(t *testing.T, builder client.TxBuilder) {
builder.SetMemo(memo)
builder.SetGasLimit(gas)
builder.SetFeeAmount(fee)
builder.SetSigBlockHeight(sbh)
err := builder.SetMsgs(msg)
require.NoError(t, err)
err = builder.SetSignatures(sig)
Expand Down
40 changes: 17 additions & 23 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func WriteGeneratedTxResponse(
}

txf := Factory{fees: br.Fees, gasPrices: br.GasPrices}.
WithSigBlockHeight(br.SigBlockHeight).
WithAccountNumber(br.AccountNumber).
WithSequence(br.Sequence).
WithGas(gasSetting.Gas).
WithGasAdjustment(gasAdj).
Expand Down Expand Up @@ -241,7 +241,6 @@ func BuildUnsignedTx(txf Factory, msgs ...sdk.Msg) (client.TxBuilder, error) {
tx.SetMemo(txf.memo)
tx.SetFeeAmount(fees)
tx.SetGasLimit(txf.gas)
tx.SetSigBlockHeight(txf.sigBlockHeight)
tx.SetTimeoutHeight(txf.TimeoutHeight())

return tx, nil
Expand Down Expand Up @@ -304,36 +303,30 @@ func CalculateGas(
return simRes, uint64(txf.GasAdjustment() * float64(simRes.GasInfo.GasUsed)), nil
}

// PrepareFactory set sig block height and account sequence to the tx factory.
// It doesn't require that the account should exist.
// If the account does not exist, then it use the zero sequence number.
// PrepareFactory ensures the account defined by ctx.GetFromAddress() exists and
// if the account number and/or the account sequence number are zero (not set),
// they will be queried for and set on the provided Factory. A new Factory with
// the updated fields will be returned.
func PrepareFactory(clientCtx client.Context, txf Factory) (Factory, error) {
from := clientCtx.GetFromAddress()
sigBlockHeight := txf.sigBlockHeight

if !clientCtx.Offline {
if sigBlockHeight == 0 {
height, err := txf.accountRetriever.GetLatestHeight(clientCtx)
if err != nil {
return txf, err
}
// `ctx.Height` of checkTx may be later by 1 block than consensus block height.
// Some cli integrated test fails because of this(sigBlockHeight = height).
sigBlockHeight = height - 1
}
if err := txf.accountRetriever.EnsureExists(clientCtx, from); err != nil {
return txf, err
}

txf = txf.WithSigBlockHeight(sigBlockHeight)

initSeq := txf.sequence
if initSeq == 0 && !clientCtx.Offline {
seq, err := txf.accountRetriever.GetAccountSequence(clientCtx, from)
initNum, initSeq := txf.accountNumber, txf.sequence
if initNum == 0 || initSeq == 0 {
num, seq, err := txf.accountRetriever.GetAccountNumberSequence(clientCtx, from)
if err != nil {
if cliError, ok := err.(*client.Error); !ok || cliError.Code != sdkerrors.ErrKeyNotFound.ABCICode() {
return txf, err
}
}

if initNum == 0 {
txf = txf.WithAccountNumber(num)
}

if initSeq == 0 {
txf = txf.WithSequence(seq)
}
Expand Down Expand Up @@ -412,8 +405,9 @@ func Sign(txf Factory, name string, txBuilder client.TxBuilder, overwriteSig boo
}
pubKey := key.GetPubKey()
signerData := authsigning.SignerData{
ChainID: txf.chainID,
Sequence: txf.sequence,
ChainID: txf.chainID,
AccountNumber: txf.accountNumber,
Sequence: txf.sequence,
}

// For SIGN_MODE_DIRECT, calling SetSignatures calls setSignerInfos on
Expand Down
6 changes: 3 additions & 3 deletions client/tx/tx_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestBuildSimTx(t *testing.T) {

txf := tx.Factory{}.
WithTxConfig(txCfg).
WithSigBlockHeight(1).
WithAccountNumber(50).
WithSequence(23).
WithFees("50stake").
WithMemo("memo").
Expand All @@ -106,7 +106,7 @@ func TestBuildSimTx(t *testing.T) {
func TestBuildUnsignedTx(t *testing.T) {
txf := tx.Factory{}.
WithTxConfig(NewTestTxConfig()).
WithSigBlockHeight(1).
WithAccountNumber(50).
WithSequence(23).
WithFees("50stake").
WithMemo("memo").
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestSign(t *testing.T) {

txfNoKeybase := tx.Factory{}.
WithTxConfig(NewTestTxConfig()).
WithSigBlockHeight(1).
WithAccountNumber(50).
WithSequence(23).
WithFees("50stake").
WithMemo("memo").
Expand Down
1 change: 0 additions & 1 deletion client/tx_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ type (
SetMsgs(msgs ...sdk.Msg) error
SetSignatures(signatures ...signingtypes.SignatureV2) error
SetMemo(memo string)
SetSigBlockHeight(sbh uint64)
SetFeeAmount(amount sdk.Coins)
SetGasLimit(limit uint64)
SetTimeoutHeight(height uint64)
Expand Down
2 changes: 1 addition & 1 deletion codec/amino_codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestAminoCodecUnpackAnyFails(t *testing.T) {

func TestAminoCodecFullDecodeAndEncode(t *testing.T) {
// This tx comes from https://github.com/cosmos/cosmos-sdk/issues/8117.
txSigned := `{"type":"lbm-sdk/StdTx","value":{"msg":[{"type":"lbm-sdk/MsgCreateValidator","value":{"description":{"moniker":"fulltest","identity":"satoshi","website":"example.com","details":"example inc"},"commission":{"rate":"0.500000000000000000","max_rate":"1.000000000000000000","max_change_rate":"0.200000000000000000"},"min_self_delegation":"1000000","delegator_address":"link120yvjfy7m2gnu9mvusrs40cxxhpt8nr3qhn8re","validator_address":"linkvaloper120yvjfy7m2gnu9mvusrs40cxxhpt8nr3jr36d2","pubkey":{"type":"ostracon/PubKeyEd25519","value":"CYrOiM3HtS7uv1B1OAkknZnFYSRpQYSYII8AtMMtev0="},"value":{"denom":"umuon","amount":"700000000"}}}],"fee":{"amount":[{"denom":"umuon","amount":"6000"}],"gas":"160000"},"signatures":[{"pub_key":{"type":"ostracon/PubKeySecp256k1","value":"AwAOXeWgNf1FjMaayrSnrOOKz+Fivr6DiI/i0x0sZCHw"},"signature":"RcnfS/u2yl7uIShTrSUlDWvsXo2p2dYu6WJC8VDVHMBLEQZWc8bsINSCjOnlsIVkUNNe1q/WCA9n3Gy1+0zhYA=="}],"sig_block_height":"0","memo":"","timeout_height":"0"}}`
txSigned := `{"type":"lbm-sdk/StdTx","value":{"msg":[{"type":"lbm-sdk/MsgCreateValidator","value":{"description":{"moniker":"fulltest","identity":"satoshi","website":"example.com","details":"example inc"},"commission":{"rate":"0.500000000000000000","max_rate":"1.000000000000000000","max_change_rate":"0.200000000000000000"},"min_self_delegation":"1000000","delegator_address":"link120yvjfy7m2gnu9mvusrs40cxxhpt8nr3qhn8re","validator_address":"linkvaloper120yvjfy7m2gnu9mvusrs40cxxhpt8nr3jr36d2","pubkey":{"type":"ostracon/PubKeyEd25519","value":"CYrOiM3HtS7uv1B1OAkknZnFYSRpQYSYII8AtMMtev0="},"value":{"denom":"umuon","amount":"700000000"}}}],"fee":{"amount":[{"denom":"umuon","amount":"6000"}],"gas":"160000"},"signatures":[{"pub_key":{"type":"ostracon/PubKeySecp256k1","value":"AwAOXeWgNf1FjMaayrSnrOOKz+Fivr6DiI/i0x0sZCHw"},"signature":"RcnfS/u2yl7uIShTrSUlDWvsXo2p2dYu6WJC8VDVHMBLEQZWc8bsINSCjOnlsIVkUNNe1q/WCA9n3Gy1+0zhYA=="}],"memo":"","timeout_height":"0"}}`
_, legacyCdc := simapp.MakeCodecs()

var tx legacytx.StdTx
Expand Down
9 changes: 5 additions & 4 deletions crypto/ledger/ledger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,16 +196,17 @@ func TestPublicKeyHDPath(t *testing.T) {
}
}

func getFakeTx() []byte {
func getFakeTx(accountNumber uint32) []byte {
tmp := fmt.Sprintf(
`{"chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"5000"},"memo":"memo","msgs":[[""]],"sequence":"6"}`)
`{"account_number":"%d","chain_id":"1234","fee":{"amount":[{"amount":"150","denom":"atom"}],"gas":"5000"},"memo":"memo","msgs":[[""]],"sequence":"6"}`,
accountNumber)

return []byte(tmp)
}

func TestSignaturesHD(t *testing.T) {
for account := uint32(0); account < 100; account += 30 {
msg := getFakeTx()
msg := getFakeTx(account)

path := *hd.NewFundraiserParams(account, sdk.CoinType, account/5)
t.Logf("Checking signature at %v --- PLEASE REVIEW AND ACCEPT IN THE DEVICE\n", path)
Expand All @@ -223,7 +224,7 @@ func TestSignaturesHD(t *testing.T) {
}

func TestRealDeviceSecp256k1(t *testing.T) {
msg := getFakeTx()
msg := getFakeTx(50)
path := *hd.NewFundraiserParams(0, sdk.CoinType, 0)
priv, err := NewPrivKeySecp256k1Unsafe(path)
require.NoError(t, err)
Expand Down
Loading