create-wallet should fail when creating a wallet with an existing salt value

[chrisli30]

Repro steps with the below commands in order

wallet
create-wallet
create-wallet 1
wallet

There shouldn’t be two rows with salt 0. Normally re-creating an existing entity should result in failure in API response.

We will need a test case in ava-sdk to cover this scenario.

Full trace:

iMac:examples mikasa$ ENV=development node example.js  wallet
Current environment is:  development
Response:
 {
  wallets: [
    {
      address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    }
  ]
}
Fetching balances from RPC provider ...
Listing smart wallet addresses for 0xc60e71bd0f2e6d8832Fea1a2d56091C48493C788 ...
 [
  {
    address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.2 ETH', '0 ASA' ]
  }
]
iMac:examples mikasa$ ENV=development node example.js  create-wallet
Current environment is:  development
A new smart wallet with salt 0 is created for 0xc60e71bd0f2e6d8832Fea1a2d56091C48493C788:
Response:
 {
  address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
  salt: '0',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
iMac:examples mikasa$ ENV=development node example.js  create-wallet 1
Current environment is:  development
A new smart wallet with salt 1 is created for 0xc60e71bd0f2e6d8832Fea1a2d56091C48493C788:
Response:
 {
  address: '0x40Baaa1657e7D4cF5d949FCAc0a88cBB4660050e',
  salt: '1',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
iMac:examples mikasa$ ENV=development node example.js  wallet
Current environment is:  development
Response:
 {
  wallets: [
    {
      address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x40Baaa1657e7D4cF5d949FCAc0a88cBB4660050e',
      salt: '1',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    }
  ]
}
Fetching balances from RPC provider ...
Listing smart wallet addresses for 0xc60e71bd0f2e6d8832Fea1a2d56091C48493C788 ...
 [
  {
    address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.2 ETH', '0 ASA' ]
  },
  {
    address: '0x40Baaa1657e7D4cF5d949FCAc0a88cBB4660050e',
    salt: '1',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0 ETH', '0 ASA' ]
  },
  {
    address: '0x6C6244dFd5d0bA3230B6600bFA380f0bB4E8AC49',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.2 ETH', '0 ASA' ]
  }
]

[v9n]

Hi @chrisli30 We agree before creating a wallet is an idempotent action. it won't failed. It won't created duplicate sale wallet.

The response was a displaying issue, due to us always pre-pend the default wallet, without checking whether user has create the default wallet.

You can try to see the behaviour:

1. If you try to create many (more than 2 salt 0 wallet) you will see exactly 2. which I will fix

Example below I call created 4 times, but it only show 2

 n example create-wallet 0
Current environment is:  development
^[[AA new smart wallet with salt 0 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
  salt: '0',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example create-wallet 0
Current environment is:  development
A new smart wallet with salt 0 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
  salt: '0',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example create-wallet 0
Current environment is:  development
A new smart wallet with salt 0 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
  salt: '0',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example create-wallet 0
Current environment is:  development
A new smart wallet with salt 0 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
  salt: '0',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example wallet
Current environment is:  development
Response:
 {
  wallets: [
    {
      address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
      salt: '13',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    }
  ]
}
Fetching balances from RPC provider ...
Listing smart wallet addresses for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5 ...
 [
  {
    address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.47 ETH', '9948.53 ASA' ]
  },
  {
    address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.47 ETH', '9948.53 ASA' ]
  },
  {
    address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
    salt: '13',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0 ETH', '0 ASA' ]
  }
]

2. If you try to create many non zer salt wallet you will see exactly only one.

❯ n example create-wallet 13
Current environment is:  development
A new smart wallet with salt 13 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
  salt: '13',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example wallet
Current environment is:  development
Response:
 {
  wallets: [
    {
      address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
      salt: '13',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    }
  ]
}
Fetching balances from RPC provider ...
Listing smart wallet addresses for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5 ...
 [
  {
    address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.47 ETH', '9948.53 ASA' ]
  },
  {
    address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
    salt: '13',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0 ETH', '0 ASA' ]
  }
]
❯ n example create-wallet 13
Current environment is:  development
A new smart wallet with salt 13 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
  salt: '13',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example create-wallet 13
Current environment is:  development
A new smart wallet with salt 13 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
  salt: '13',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example create-wallet 13
Current environment is:  development
A new smart wallet with salt 13 is created for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5:
Response:
 {
  address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
  salt: '13',
  factory_address: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
}
❯ n example wallet
Current environment is:  development
Response:
 {
  wallets: [
    {
      address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
      salt: '0',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    },
    {
      address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
      salt: '13',
      factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7'
    }
  ]
}
Fetching balances from RPC provider ...
Listing smart wallet addresses for 0xe272b72E51a5bF8cB720fc6D6DF164a4D5E321C5 ...
 [
  {
    address: '0x5Df343de7d99fd64b2479189692C1dAb8f46184a',
    salt: '0',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0.47 ETH', '9948.53 ASA' ]
  },
  {
    address: '0x7b7cd9b7637ac8FA692F34A05DdBc24D9498959c',
    salt: '13',
    factory: '0x29adA1b5217242DEaBB142BC3b1bCfFdd56008e7',
    balances: [ '0 ETH', '0 ASA' ]
  }
]

3. If you try to create more than

[chrisli30]

I understand that we agreed on the silent succeed behavior of createWallet, but that doesn’t seem to comply with the norm of a create operation, which is usually not an idempotent action.

What do you think of renaming the operation to getWallet()? My reasoning is that, a Get would never fail, and in reality this is a get operation, as the salt-address binding have already been decided.

Note: #39 has added tests around this scenario.

[v9n]

this is done.

[v9n]

I understand that we agreed on the silent succeed behavior of createWallet, but that doesn’t seem to comply with the norm of a create operation, which is usually not an idempotent action.

What do you think of renaming the operation to getWallet()? My reasoning is that, a Get would never fail, and in reality this is a get operation, as the salt-address binding have already been decided.

I disagree but this is a small change to make so I can change quickly.

[v9n]

Fixed in #59