From cff6bcfb31089b5771aa244438618d9c39658552 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Sat, 23 May 2015 02:05:56 -0400 Subject: [PATCH] copy entry on get, use strings for name/data, uint64 for expires --- rpc/core/names.go | 2 +- rpc/core_client/client_methods.go | 6 +++--- rpc/test/helpers.go | 4 ++-- rpc/test/tests.go | 6 +++--- state/block_cache.go | 16 ++++++++-------- state/execution.go | 25 +++++++++++++++---------- state/state.go | 7 +++---- state/state_test.go | 17 +++++++++++++---- types/names.go | 13 +++++++++---- types/tx.go | 17 +++++++++++++++-- types/tx_utils.go | 4 ++-- 11 files changed, 74 insertions(+), 43 deletions(-) diff --git a/rpc/core/names.go b/rpc/core/names.go index 928afb152..b73662d29 100644 --- a/rpc/core/names.go +++ b/rpc/core/names.go @@ -7,7 +7,7 @@ import ( ) // XXX: need we be careful about rendering bytes as string or is that their problem ? -func NameRegEntry(name []byte) (*ctypes.ResponseNameRegEntry, error) { +func NameRegEntry(name string) (*ctypes.ResponseNameRegEntry, error) { st := consensusState.GetState() // performs a copy entry := st.GetNameRegEntry(name) if entry == nil { diff --git a/rpc/core_client/client_methods.go b/rpc/core_client/client_methods.go index cf3804dcd..a5cbefbd4 100644 --- a/rpc/core_client/client_methods.go +++ b/rpc/core_client/client_methods.go @@ -28,7 +28,7 @@ type Client interface { ListAccounts() (*ctypes.ResponseListAccounts, error) ListUnconfirmedTxs() (*ctypes.ResponseListUnconfirmedTxs, error) ListValidators() (*ctypes.ResponseListValidators, error) - NameRegEntry(name []byte) (*ctypes.ResponseNameRegEntry, error) + NameRegEntry(name string) (*ctypes.ResponseNameRegEntry, error) NetInfo() (*ctypes.ResponseNetInfo, error) SignTx(tx types.Tx, privAccounts []*account.PrivAccount) (*ctypes.ResponseSignTx, error) Status() (*ctypes.ResponseStatus, error) @@ -454,7 +454,7 @@ func (c *ClientHTTP) ListValidators() (*ctypes.ResponseListValidators, error) { return response.Result, nil } -func (c *ClientHTTP) NameRegEntry(name []byte) (*ctypes.ResponseNameRegEntry, error) { +func (c *ClientHTTP) NameRegEntry(name string) (*ctypes.ResponseNameRegEntry, error) { values, err := argsToURLValues([]string{"name"}, name) if err != nil { return nil, err @@ -952,7 +952,7 @@ func (c *ClientJSON) ListValidators() (*ctypes.ResponseListValidators, error) { return response.Result, nil } -func (c *ClientJSON) NameRegEntry(name []byte) (*ctypes.ResponseNameRegEntry, error) { +func (c *ClientJSON) NameRegEntry(name string) (*ctypes.ResponseNameRegEntry, error) { request := rpctypes.RPCRequest{ JSONRPC: "2.0", Method: reverseFuncMap["NameRegEntry"], diff --git a/rpc/test/helpers.go b/rpc/test/helpers.go index bea0253df..5e01a30ce 100644 --- a/rpc/test/helpers.go +++ b/rpc/test/helpers.go @@ -120,7 +120,7 @@ func makeDefaultCallTx(t *testing.T, typ string, addr, code []byte, amt, gasLim, return tx } -func makeDefaultNameTx(t *testing.T, typ string, name, value []byte, amt, fee uint64) *types.NameTx { +func makeDefaultNameTx(t *testing.T, typ string, name, value string, amt, fee uint64) *types.NameTx { nonce := getNonce(t, typ, user[0].Address) tx := types.NewNameTxWithNonce(user[0].PubKey, name, value, amt, fee, nonce) tx.Sign(user[0]) @@ -218,7 +218,7 @@ func callContract(t *testing.T, client cclient.Client, address, data, expected [ } // get the namereg entry -func getNameRegEntry(t *testing.T, typ string, name []byte) *types.NameRegEntry { +func getNameRegEntry(t *testing.T, typ string, name string) *types.NameRegEntry { client := clients[typ] r, err := client.NameRegEntry(name) if err != nil { diff --git a/rpc/test/tests.go b/rpc/test/tests.go index 8f6359d4b..19a5b1b50 100644 --- a/rpc/test/tests.go +++ b/rpc/test/tests.go @@ -202,8 +202,8 @@ func testNameReg(t *testing.T, typ string) { amt, fee := uint64(6969), uint64(1000) // since entries ought to be unique and these run against different clients, we append the typ - name := []byte("ye-old-domain-name-" + typ) - data := []byte("these are amongst the things I wish to bestow upon the youth of generations come: a safe supply of honey, and a better money. For what else shall they need?") + name := "ye-old-domain-name-" + typ + data := "these are amongst the things I wish to bestow upon the youth of generations come: a safe supply of honey, and a better money. For what else shall they need?" tx := makeDefaultNameTx(t, typ, name, data, amt, fee) broadcastTx(t, typ, tx) @@ -215,7 +215,7 @@ func testNameReg(t *testing.T, typ string) { entry := getNameRegEntry(t, typ, name) - if bytes.Compare(entry.Data, data) != 0 { + if entry.Data != data { t.Fatal(fmt.Sprintf("Got %s, expected %s", entry.Data, data)) } diff --git a/state/block_cache.go b/state/block_cache.go index 102d26253..53278aa19 100644 --- a/state/block_cache.go +++ b/state/block_cache.go @@ -128,15 +128,15 @@ func (cache *BlockCache) SetStorage(addr Word256, key Word256, value Word256) { //------------------------------------- // BlockCache.names -func (cache *BlockCache) GetNameRegEntry(name []byte) *types.NameRegEntry { - entry, removed, _ := cache.names[string(name)].unpack() +func (cache *BlockCache) GetNameRegEntry(name string) *types.NameRegEntry { + entry, removed, _ := cache.names[name].unpack() if removed { return nil } else if entry != nil { return entry } else { entry = cache.backend.GetNameRegEntry(name) - cache.names[string(name)] = nameInfo{entry, false, false} + cache.names[name] = nameInfo{entry, false, false} return entry } } @@ -144,22 +144,22 @@ func (cache *BlockCache) GetNameRegEntry(name []byte) *types.NameRegEntry { func (cache *BlockCache) UpdateNameRegEntry(entry *types.NameRegEntry) { name := entry.Name // SANITY CHECK - _, removed, _ := cache.names[string(name)].unpack() + _, removed, _ := cache.names[name].unpack() if removed { panic("UpdateNameRegEntry on a removed name") } // SANITY CHECK END - cache.names[string(name)] = nameInfo{entry, false, true} + cache.names[name] = nameInfo{entry, false, true} } -func (cache *BlockCache) RemoveNameRegEntry(name []byte) { +func (cache *BlockCache) RemoveNameRegEntry(name string) { // SANITY CHECK - _, removed, _ := cache.names[string(name)].unpack() + _, removed, _ := cache.names[name].unpack() if removed { panic("RemoveNameRegEntry on a removed entry") } // SANITY CHECK END - cache.names[string(name)] = nameInfo{nil, true, false} + cache.names[name] = nameInfo{nil, true, false} } // BlockCache.names diff --git a/state/execution.go b/state/execution.go index a9f3ca823..dab938f39 100644 --- a/state/execution.go +++ b/state/execution.go @@ -504,13 +504,18 @@ func ExecTx(blockCache *BlockCache, tx_ types.Tx, runCall bool, evc events.Firea return types.ErrTxInsufficientFunds } + // validate the input strings + if !tx.Validate() { + log.Debug(Fmt("Invalid characters present in NameTx name (%s) or data (%s)", tx.Name, tx.Data)) + return types.ErrTxInvalidString + } + value := tx.Input.Amount - tx.Fee // let's say cost of a name for one block is len(data) + 32 - // TODO: the casting is dangerous and things can overflow (below)! - // we should make LastBlockHeight a uint64 - costPerBlock := uint(len(tx.Data) + 32) - expiresIn := uint(value / uint64(costPerBlock)) + costPerBlock := uint64(len(tx.Data) + 32) + expiresIn := value / uint64(costPerBlock) + lastBlockHeight := uint64(_s.LastBlockHeight) // check if the name exists entry := blockCache.GetNameRegEntry(tx.Name) @@ -518,7 +523,7 @@ func ExecTx(blockCache *BlockCache, tx_ types.Tx, runCall bool, evc events.Firea if entry != nil { var expired bool // if the entry already exists, and hasn't expired, we must be owner - if entry.Expires > _s.LastBlockHeight { + if entry.Expires > lastBlockHeight { // ensure we are owner if bytes.Compare(entry.Owner, tx.Input.Address) != 0 { log.Debug(Fmt("Sender %X is trying to update a name (%s) for which he is not owner", tx.Input.Address, tx.Name)) @@ -537,14 +542,14 @@ func ExecTx(blockCache *BlockCache, tx_ types.Tx, runCall bool, evc events.Firea // update the entry by bumping the expiry // and changing the data if expired { - entry.Expires = _s.LastBlockHeight + expiresIn + entry.Expires = lastBlockHeight + expiresIn } else { // since the size of the data may have changed // we use the total amount of "credit" - credit := uint64((entry.Expires - _s.LastBlockHeight) * uint(len(entry.Data))) + credit := (entry.Expires - lastBlockHeight) * uint64(len(entry.Data)) credit += value - expiresIn = uint(credit) / costPerBlock - entry.Expires = _s.LastBlockHeight + expiresIn + expiresIn = credit / costPerBlock + entry.Expires = lastBlockHeight + expiresIn } entry.Data = tx.Data blockCache.UpdateNameRegEntry(entry) @@ -558,7 +563,7 @@ func ExecTx(blockCache *BlockCache, tx_ types.Tx, runCall bool, evc events.Firea Name: tx.Name, Owner: tx.Input.Address, Data: tx.Data, - Expires: _s.LastBlockHeight + expiresIn, + Expires: lastBlockHeight + expiresIn, } blockCache.UpdateNameRegEntry(entry) } diff --git a/state/state.go b/state/state.go index e5bc863ea..a36fbff21 100644 --- a/state/state.go +++ b/state/state.go @@ -283,21 +283,20 @@ func (s *State) LoadStorage(hash []byte) (storage merkle.Tree) { //------------------------------------- // State.nameReg -func (s *State) GetNameRegEntry(name []byte) *types.NameRegEntry { +func (s *State) GetNameRegEntry(name string) *types.NameRegEntry { _, value := s.nameReg.Get(name) if value == nil { return nil } entry := value.(*types.NameRegEntry) - // XXX: do we need to copy? - return entry + return entry.Copy() } func (s *State) UpdateNameRegEntry(entry *types.NameRegEntry) bool { return s.nameReg.Set(entry.Name, entry) } -func (s *State) RemoveNameRegEntry(name []byte) bool { +func (s *State) RemoveNameRegEntry(name string) bool { _, removed := s.nameReg.Remove(name) return removed } diff --git a/state/state_test.go b/state/state_test.go index 6804ca0c4..3c4d70c73 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -304,8 +304,8 @@ func TestTxs(t *testing.T) { // NameTx. { - entryName := []byte("satoshi") - entryData := []byte(` + entryName := "satoshi" + entryData := ` A purely peer-to-peer version of electronic cash would allow online payments to be sent directly from one party to another without going through a financial institution. Digital signatures provide part of the solution, but the main @@ -319,7 +319,7 @@ long as a majority of CPU power is controlled by nodes that are not cooperating attack the network, they'll generate the longest chain and outpace attackers. The network itself requires minimal structure. Messages are broadcast on a best effort basis, and nodes can leave and rejoin the network at will, accepting the longest -proof-of-work chain as proof of what happened while they were gone `) +proof-of-work chain as proof of what happened while they were gone ` entryAmount := uint64(10000) state := state.Copy() @@ -348,9 +348,18 @@ proof-of-work chain as proof of what happened while they were gone `) if entry == nil { t.Errorf("Expected an entry but got nil") } - if bytes.Compare(entry.Data, entryData) != 0 { + if entry.Data != entryData { t.Errorf("Wrong data stored") } + + // test a bad string + tx.Data = string([]byte{0, 1, 2, 3, 127, 128, 129, 200, 251}) + tx.Input.Sequence += 1 + tx.Input.Signature = privAccounts[0].Sign(tx) + err = execTxWithState(state, tx, true) + if err != types.ErrTxInvalidString { + t.Errorf("Expected invalid string error. Got: %s", err.Error()) + } } // BondTx. diff --git a/types/names.go b/types/names.go index 2fbb16da8..e72cc8e16 100644 --- a/types/names.go +++ b/types/names.go @@ -1,8 +1,13 @@ package types type NameRegEntry struct { - Name []byte // registered name for the entry - Owner []byte // address that created the entry - Data []byte // binary encoded byte array - Expires uint // block at which this entry expires + Name string `json:"name"` // registered name for the entry + Owner []byte `json:"owner"` // address that created the entry + Data string `json:"data"` // binary encoded byte array + Expires uint64 `json:"expires"` // block at which this entry expires +} + +func (entry *NameRegEntry) Copy() *NameRegEntry { + entryCopy := *entry + return &entryCopy } diff --git a/types/tx.go b/types/tx.go index 06f90d796..15a61cf89 100644 --- a/types/tx.go +++ b/types/tx.go @@ -3,6 +3,7 @@ package types import ( "errors" "io" + "regexp" "github.com/tendermint/tendermint/account" "github.com/tendermint/tendermint/binary" @@ -18,6 +19,7 @@ var ( ErrTxUnknownPubKey = errors.New("Error unknown pubkey") ErrTxInvalidPubKey = errors.New("Error invalid pubkey") ErrTxInvalidSignature = errors.New("Error invalid signature") + ErrTxInvalidString = errors.New("Error invalid string") ) type ErrTxInvalidSequence struct { @@ -183,8 +185,8 @@ func (tx *CallTx) String() string { type NameTx struct { Input *TxInput `json:"input"` - Name []byte `json:"name"` - Data []byte `json:"data"` + Name string `json:"name"` + Data string `json:"data"` Fee uint64 `json:"fee"` } @@ -197,6 +199,17 @@ func (tx *NameTx) WriteSignBytes(w io.Writer, n *int64, err *error) { binary.WriteTo([]byte(`}]}`), w, n, err) } +// alphanum, underscore, forward slash +var regexpAlphaNum, _ = regexp.Compile("^[a-zA-Z0-9_/]*$") + +// anything you might find in a json +var regexpJSON, err = regexp.Compile(`^[a-zA-Z0-9_/ \-"':,\n\t.{}()\[\]]*$`) + +func (tx *NameTx) Validate() bool { + // Name should be alphanum and Data should be like JSON + return regexpAlphaNum.Match([]byte(tx.Name)) && regexpJSON.Match([]byte(tx.Data)) +} + func (tx *NameTx) String() string { return Fmt("NameTx{%v -> %s: %s}", tx.Input, tx.Name, tx.Data) } diff --git a/types/tx_utils.go b/types/tx_utils.go index beae2e861..a7228c7b4 100644 --- a/types/tx_utils.go +++ b/types/tx_utils.go @@ -106,7 +106,7 @@ func (tx *CallTx) Sign(chainID string, privAccount *account.PrivAccount) { //---------------------------------------------------------------------------- // NameTx interface for creating tx -func NewNameTx(st AccountGetter, from account.PubKey, name, data []byte, amt, fee uint64) (*NameTx, error) { +func NewNameTx(st AccountGetter, from account.PubKey, name, data string, amt, fee uint64) (*NameTx, error) { addr := from.Address() acc := st.GetAccount(addr) if acc == nil { @@ -117,7 +117,7 @@ func NewNameTx(st AccountGetter, from account.PubKey, name, data []byte, amt, fe return NewNameTxWithNonce(from, name, data, amt, fee, nonce), nil } -func NewNameTxWithNonce(from account.PubKey, name, data []byte, amt, fee, nonce uint64) *NameTx { +func NewNameTxWithNonce(from account.PubKey, name, data string, amt, fee, nonce uint64) *NameTx { addr := from.Address() input := &TxInput{ Address: addr,