Skip to content

Commit

Permalink
accouts: composite primary key over id and id_type (#1279)
Browse files Browse the repository at this point in the history 
This makes no functional or user visible changes, but it does alter how the account store tables work.

With

```json
"alloc": [
    {
      "id": "0xAfFDC06cF34aFD7D5801A13d48C92AD39609901D",
      "key_type": "secp256k1",
      "amount": 10000000000000000000000000000000000000000000
    }
  ],
```

we had

```
# table kwild_accts.accounts ;
┌─[ RECORD 1 ]──────────────────────────────────────────────────────────────────────────────┐
│ identifier │ \x14000000affdc06cf34afd7d5801a13d48c92ad39609901d09000000736563703235366b31 │
│ balance    │ 10000000000000000000000000000000000000000000                                 │
│ nonce      │ 0                                                                            │
└────────────┴──────────────────────────────────────────────────────────────────────────────┘
```

With this change we have

```
 # table kwild_accts.accounts ;
┌─[ RECORD 1 ]──────────────────────────────────────────────┐
│ identifier │ \xaffdc06cf34afd7d5801a13d48c92ad39609901d   │
│ id_type    │ 0                                            │
│ balance    │ 10000000000000000000000000000000000000000000 │
│ nonce      │ 0                                            │
└────────────┴──────────────────────────────────────────────┘
```

So more compact and readable for debugging.

Summarizing some good discussion in Slack about this:

- The owner of a given secp256k1 pubkey can (with difficulty) transact
  with different auth types where one indicates eth address as sender
  and the other indicates full pubkey, and these are unfortunately
  considered different accounts.
- If there were a hypothetical third secp256k1 auth type that also uses
  full pubkey as id/sender (just some different message digest
  function), that will end up the same account.  This was the reason
  behind using key type rather than auth type in the accounts store.
- The above effects are inconsistent.
- We cannot get the pubkey (practically) for eth web3 wallets, so we
  can't change that.
- It's unlikely a user will fall into this pitfall, since it takes real
  effort to make a transaction from a browser / node app that would use
  the non-personal-sign / direct auth type and the full pubkey.  They'd
  need a kwil-js or kwil-cli fork, since these tools are specifically
  designed to match and use eth addresses.
- We could consider linking/aliasing certain account types, for the
  purpose of nonce and balance checks via only 0x adddress.  This can be
  done after the first transaction from a user that contains a valid
  signature that allows the recovered public key to be recorded in this
  lookup table.  Balance lookups by eth address or full pubkey will
  work.  This could be a mess of an implementation.

We decided on no changes for now.
  • Loading branch information
@jchappelow
jchappelow authored Jan 24, 2025
1 parent f108119 commit 3be3405
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 52 deletions.
8 changes: 8 additions & 0 deletions core/crypto/keys.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ func ParseKeyType(s string) (KeyType, error) {
return "", fmt.Errorf("unknown key type: %s", s)
}

func ParseKeyTypeID(id uint32) (KeyType, error) {
kt, ok := encodingIDs[id]
if !ok {
return "", fmt.Errorf("unknown key type ID: %d", id)
}
return kt, nil
}

func UnmarshalPublicKey(b []byte, kt KeyType) (PublicKey, error) {
kd, ok := keyTypes[kt]
if !ok {
Expand Down
39 changes: 20 additions & 19 deletions node/accounts/accounts.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ package accounts

import (
"context"
"encoding/hex"
"errors"
"fmt"
"math/big"
"sync"

"github.com/kwilteam/kwil-db/core/crypto"
"github.com/kwilteam/kwil-db/core/log"
"github.com/kwilteam/kwil-db/core/types"
"github.com/kwilteam/kwil-db/node/types/sql"
Expand Down Expand Up @@ -85,15 +87,12 @@ func (a *Accounts) getAccount(ctx context.Context, tx sql.Executor, account *typ
a.mtx.Lock()
defer a.mtx.Unlock()

acctID, err := account.MarshalBinary()
if err != nil {
return nil, err
}
mapKey := acctMapKey(account)

var ok bool
if uncommitted {
// Check in the updates first to see if the account has been updated in the current block
acct, ok = a.updates[string(acctID)]
acct, ok = a.updates[mapKey]
if ok {
return &types.Account{
ID: account,
Expand All @@ -104,7 +103,7 @@ func (a *Accounts) getAccount(ctx context.Context, tx sql.Executor, account *typ
}

// Check in the records to see if the account has been read before
acct, ok = a.records[string(acctID)]
acct, ok = a.records[mapKey]
if ok {
return &types.Account{
ID: account,
Expand All @@ -119,7 +118,7 @@ func (a *Accounts) getAccount(ctx context.Context, tx sql.Executor, account *typ
}

// Add the account to the in-memory cache
a.records[string(acctID)] = &types.Account{
a.records[mapKey] = &types.Account{
ID: account,
Balance: big.NewInt(0).Set(acct.Balance),
Nonce: acct.Nonce,
Expand Down Expand Up @@ -329,21 +328,24 @@ func (a *Accounts) Rollback() {
a.spends = nil
}

func acctMapKey(account *types.AccountID) string {
return hex.EncodeToString(account.Identifier) + "#" + account.KeyType.String()
}

func (a *Accounts) createAccount(ctx context.Context, tx sql.Executor, account *types.AccountID, amt *big.Int, nonce int64) error {
accountID, err := account.MarshalBinary()
if err != nil {
return err
kd, ok := crypto.KeyTypeDefinition(account.KeyType)
if !ok {
return fmt.Errorf("invalid key type: %s", account.KeyType)
}

if err := createAccount(ctx, tx, accountID, amt, nonce); err != nil {
if err := createAccount(ctx, tx, account.Identifier, kd.EncodeFlag(), amt, nonce); err != nil {
return err
}

// Record the account creation in the updates
a.mtx.Lock()
defer a.mtx.Unlock()

a.updates[string(accountID)] = &types.Account{
a.updates[acctMapKey(account)] = &types.Account{
ID: account,
Balance: big.NewInt(0).Set(amt),
Nonce: nonce,
Expand All @@ -353,20 +355,19 @@ func (a *Accounts) createAccount(ctx context.Context, tx sql.Executor, account *
}

func (a *Accounts) updateAccount(ctx context.Context, tx sql.Executor, account *types.AccountID, amount *big.Int, nonce int64) error {
accountID, err := account.MarshalBinary()
if err != nil {
return err
kd, ok := crypto.KeyTypeDefinition(account.KeyType)
if !ok {
return fmt.Errorf("invalid key type: %s", account.KeyType)
}

if err := updateAccount(ctx, tx, accountID, amount, nonce); err != nil {
if err := updateAccount(ctx, tx, account.Identifier, kd.EncodeFlag(), amount, nonce); err != nil {
return err
}

// Record the account update in the updates
a.mtx.Lock()
defer a.mtx.Unlock()

a.updates[string(accountID)] = &types.Account{
a.updates[acctMapKey(account)] = &types.Account{
ID: account,
Balance: big.NewInt(0).Set(amount),
Nonce: nonce,
Expand Down
55 changes: 38 additions & 17 deletions node/accounts/accounts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,25 @@ func (m *mockDB) Execute(ctx context.Context, stmt string, args ...any) (*sql.Re
switch stmt {
case sqlCreateAccount: // via createAccount and createAccountWithNonce
acctID := args[0].([]byte)
bal, ok := big.NewInt(0).SetString(args[1].(string), 10)
idType := args[1].(uint32)
bal, ok := big.NewInt(0).SetString(args[2].(string), 10)
if !ok {
return nil, errors.New("not a string balance")
}

acct := &types.AccountID{}
err := acct.UnmarshalBinary(acctID)
keyType, err := crypto.ParseKeyTypeID(idType)
if err != nil {
return nil, err
}

m.accts[string(acctID)] = &types.Account{
acct := &types.AccountID{
Identifier: acctID,
KeyType: keyType,
}

m.accts[acctMapKey(acct)] = &types.Account{
ID: acct,
Nonce: args[2].(int64),
Nonce: args[3].(int64),
Balance: bal,
}
return &sql.ResultSet{
Expand All @@ -74,9 +79,19 @@ func (m *mockDB) Execute(ctx context.Context, stmt string, args ...any) (*sql.Re
if !ok {
return nil, errors.New("not a string balance")
}
acctID, nonce := args[2].([]byte), args[1].(int64)
nonce := args[1].(int64)

acctID, idType := args[2].([]byte), args[3].(uint32)
keyType, err := crypto.ParseKeyTypeID(idType)
if err != nil {
return nil, err
}
acct := &types.AccountID{
Identifier: acctID,
KeyType: keyType,
}

acct, ok := m.accts[string(acctID)]
account, ok := m.accts[acctMapKey(acct)]
if !ok {
return &sql.ResultSet{
Status: sql.CommandTag{
Expand All @@ -86,8 +101,8 @@ func (m *mockDB) Execute(ctx context.Context, stmt string, args ...any) (*sql.Re
}, nil
}

acct.Balance = bal
acct.Nonce = nonce
account.Balance = bal
account.Nonce = nonce

return &sql.ResultSet{
Status: sql.CommandTag{
Expand All @@ -97,15 +112,23 @@ func (m *mockDB) Execute(ctx context.Context, stmt string, args ...any) (*sql.Re
}, nil
case sqlGetAccount: // via getAccount
m.accessCnt++
acctID := args[0].([]byte)
acct, ok := m.accts[string(acctID)]
acctID, idType := args[0].([]byte), args[1].(uint32)
keyType, err := crypto.ParseKeyTypeID(idType)
if err != nil {
return nil, err
}
acct := &types.AccountID{
Identifier: acctID,
KeyType: keyType,
}
account, ok := m.accts[acctMapKey(acct)]
if !ok {
return &sql.ResultSet{}, nil // not ErrNoRows since we don't use Scan in pg
}
return &sql.ResultSet{
Columns: []string{"balance", "nonce"},
Rows: [][]any{
{acct.Balance.String(), acct.Nonce},
{account.Balance.String(), account.Nonce},
},
}, nil
default:
Expand Down Expand Up @@ -162,13 +185,11 @@ var acctsTestCases = []acctsTestCase{
// first credit, access db
verifyDBAccessCount(t, c, 1, skip)

acctID1, err := account1.MarshalBinary()
require.NoError(t, err)

_, ok := a.records[string(acctID1)]
key := acctMapKey(account1)
_, ok := a.records[key]
require.False(t, ok)

acct, ok := a.updates[string(acctID1)]
acct, ok := a.updates[key]
require.True(t, ok)
assert.Equal(t, int64(100), acct.Balance.Int64())

Expand Down
32 changes: 16 additions & 16 deletions node/accounts/sql.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"math/big"

"github.com/kwilteam/kwil-db/core/crypto"
"github.com/kwilteam/kwil-db/core/types"
"github.com/kwilteam/kwil-db/node/types/sql"
)
Expand All @@ -16,19 +17,19 @@ const (
accountStoreVersion = 0

sqlInitTables = `CREATE TABLE IF NOT EXISTS ` + schemaName + `.accounts (
identifier BYTEA PRIMARY KEY,
identifier BYTEA NOT NULL,
id_type INT4 NOT NULL,
balance TEXT NOT NULL, -- consider: NUMERIC(32) for uint256 and pgx.Numeric will handle it and provide a *big.Int field
nonce BIGINT NOT NULL -- a.k.a. INT8
nonce BIGINT NOT NULL, -- a.k.a. INT8
PRIMARY KEY(identifier, id_type)
);`

sqlCreateAccount = `INSERT INTO ` + schemaName + `.accounts (identifier, balance, nonce) VALUES ($1, $2, $3)`

// sqlCreateAccountIfNotExists = `INSERT INTO ` + schemaName + `.accounts (identifier, balance, nonce) VALUES ($1, $2, $3) ON CONFLICT DO NOTHING`
sqlCreateAccount = `INSERT INTO ` + schemaName + `.accounts (identifier, id_type, balance, nonce) VALUES ($1, $2, $3, $4)`

sqlUpdateAccount = `UPDATE ` + schemaName + `.accounts SET balance = $1, nonce = $2
WHERE identifier = $3`
WHERE identifier = $3 AND id_type = $4`

sqlGetAccount = `SELECT balance, nonce FROM ` + schemaName + `.accounts WHERE identifier = $1`
sqlGetAccount = `SELECT balance, nonce FROM ` + schemaName + `.accounts WHERE identifier = $1 AND id_type = $2`
)

func initTables(ctx context.Context, tx sql.DB) error {
Expand All @@ -41,27 +42,26 @@ func initTables(ctx context.Context, tx sql.DB) error {
}

// updateAccount updates the balance and nonce of an account.
func updateAccount(ctx context.Context, db sql.Executor, acctID []byte, amount *big.Int, nonce int64) error {
_, err := db.Execute(ctx, sqlUpdateAccount, amount.String(), nonce, acctID)
func updateAccount(ctx context.Context, db sql.Executor, acctID []byte, acctType uint32, amount *big.Int, nonce int64) error {
_, err := db.Execute(ctx, sqlUpdateAccount, amount.String(), nonce, acctID, acctType)
return err
}

// createAccount creates an account with the given identifier and
// initial balance. The nonce will be set to 0.
func createAccount(ctx context.Context, db sql.Executor, acctID []byte, amt *big.Int, nonce int64) error {
_, err := db.Execute(ctx, sqlCreateAccount, acctID, amt.String(), nonce)
func createAccount(ctx context.Context, db sql.Executor, acctID []byte, acctType uint32, amt *big.Int, nonce int64) error {
_, err := db.Execute(ctx, sqlCreateAccount, acctID, acctType, amt.String(), nonce)
return err
}

// getAccount retrieves an account from the database.
// if the account is not found, it returns nil, ErrAccountNotFound.
func getAccount(ctx context.Context, db sql.Executor, account *types.AccountID) (*types.Account, error) {
accountID, err := account.MarshalBinary()
if err != nil {
return nil, err
kd, ok := crypto.KeyTypeDefinition(account.KeyType)
if !ok {
return nil, fmt.Errorf("invalid key type: %s", account.KeyType)
}

results, err := db.Execute(ctx, sqlGetAccount, accountID)
results, err := db.Execute(ctx, sqlGetAccount, []byte(account.Identifier), kd.EncodeFlag())
if err != nil {
return nil, err
}
Expand Down

0 comments on commit 3be3405

Please sign in to comment.