[DO NOT MERGE] new accounts table #49

rudolfix · 2021-11-15T08:30:52Z

this PR

introduces accounts tables to make the mock compatible with new trading engine
changes number format to NUMERIC in postgres and BigInt scale to 10^18 (form 10^4) - required for blockchain integration

rudolfix

also please use enum/constants for the list of namespaces (in javascript, not in postgres). so we know what we use

rudolfix · 2021-11-17T08:32:01Z

erc20_moc/Bet.noblock.js

@@ -37,7 +37,7 @@ class Bet {
  constructor(betId, outcomes) {
    this.betId = betId;
    this.fee = 0.01;
-    this.walletId = WALLET_PREFIX + betId;
+    this.walletId = betId;


for backward compatibility please do not remove WALLET_PREFIX, we'll do it later. not everything at once :)

rudolfix · 2021-11-17T08:33:24Z

erc20_moc/Bet.noblock.js

@@ -234,21 +236,20 @@ class Bet {
      }
      // TODO: implement a method to transfer several tokens in one call (like batch transfer)
      // get the collateral from provides
-      await this.collateralToken.transferChain(dbClient, provider, this.walletId, amount);
+      await this.collateralToken.transferChain(dbClient, provider, this.walletId, 'eth', 'bet', amount);


why are we using eth namespace? the WFAIR for the liqudiity as WFAIR for betting should be in usr. @SamirHodzic please verify

rudolfix · 2021-11-17T08:36:06Z

erc20_moc/Erc20.noblock.js

@@ -86,15 +85,14 @@ class ERC20 {
   * @param amount {bigint}
   * @returns {Promise<void>}
   */
-  transferChain = async (dbClient, sender, receiver, amount) => {
+  transferChain = async (dbClient, sender, receiver, senderNamespace, receiverNamespace, amount, _symbol) => {


why you need the override for symbol? something is wrong here. you transfer this.symbol and that must be used consistently

rudolfix · 2021-11-17T08:36:45Z

erc20_moc/Bet.noblock.js

      }

      // send back the tokens to make the market (new market) / keep the price constant (further adds)
      if (sendBacksAmounts) {
        for (const [o, a] of Object.entries(sendBacksAmounts)) {
          if (a > 0n) {
            const token = new ERC20(this.getOutcomeKey(o));
-            // console.log(`TX back ${o}: ${a}`);
-            await token.transferChain(dbClient, this.walletId, provider, a);
+            await token.transferChain(dbClient, this.walletId, provider, 'bet', 'eth', a, 'WFAIR');


here you transfer outcome tokens. outcome tokens exist only within bet namespace. what eth is doing here?

rudolfix · 2021-11-17T08:37:28Z

erc20_moc/Bet.noblock.js

@@ -503,21 +504,23 @@ class Bet {
        throw new NoWeb3Exception('Minimum buy amount not reached');
      }

-      await this.collateralToken.transferChain(dbClient, buyer, this.walletId, investmentAmount);
+      await this.collateralToken.transferChain(dbClient, buyer, this.walletId, 'usr', 'bet', investmentAmount);


ok, this is correct!

rudolfix · 2021-11-17T08:42:43Z

test/Erc20.noblock.test.js


  expect(await WFAIR.balanceOf(testMintWallet)).toBe(tokensToMint);
+
+  await WFAIR.burn(testMintWallet, 'usr', tokensToMint);
 });

 test('Transfer Tokens', async () => {


also test failed transfer due to not enough balance

rudolfix · 2021-11-17T08:43:52Z

utils/db_helper.js

@@ -26,7 +26,20 @@ const CREATE_CASINO_MATCHES =
  'CREATE TABLE IF NOT EXISTS casino_matches (ID SERIAL PRIMARY KEY, gameId varchar(255) NOT NULL, gameHash varchar(255), crashFactor decimal NOT NULL, gameLengthInSeconds INT, amountInvestedSum bigint, amountRewardedSum bigint, numTrades INT, numcashouts INT, created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP)';
 const CREATE_CASINO_TRADES =
  'CREATE TABLE IF NOT EXISTS casino_trades (ID SERIAL PRIMARY KEY, userId varchar(255) NOT NULL, crashFactor decimal NOT NULL, stakedAmount bigint NOT NULL, state smallint NOT NULL, gameHash varchar(255), gameId varchar(255), created_at TIMESTAMP WITH TIME ZONE DEFAULT CURRENT_TIMESTAMP, riskFactor decimal, game_match int, CONSTRAINT fk_game_match FOREIGN KEY (game_match) REFERENCES casino_matches(ID));';
-
+const CREATE_ACCOUNT_NAMESPACE_ENUM = `CREATE TYPE account_namespace_enum AS ENUM('usr', 'eth', 'bet', 'tdl', 'cas')`;


are we still using enums? those tables are not compatible with a new trading engine. now we have strings.

you're right, it's varchar now

rudolfix · 2021-11-17T08:44:19Z

utils/db_helper.js

-
+const CREATE_ACCOUNT_NAMESPACE_ENUM = `CREATE TYPE account_namespace_enum AS ENUM('usr', 'eth', 'bet', 'tdl', 'cas')`;
+const CREATE_ACCOUNTS =
+  `CREATE TABLE IF NOT EXISTS "account" ("owner_account" character varying NOT NULL, "account_namespace" "public"."account_namespace_enum" NOT NULL, "symbol" character varying NOT NULL, "balance" numeric(18,0) NOT NULL, CONSTRAINT "PK_8ec3dedb1ee17a8630a7c57b0f9" PRIMARY KEY ("owner_account", "account_namespace", "symbol"));` +


this is very wrong. test code in the production code? why do you need to create accounts?

This is supposed to be only in test/db/db_helper not sure how it ended up here.

rudolfix · 2021-11-17T08:45:35Z

erc20_moc/Erc20.noblock.js

-        amount
-      );
-
-      await insertTransaction(


where is transaction log? why it's removed?

are we supposed to insert a new record in transactions table each time we make an internal transaction, because we're not doing it in trading-engine atm?

rudolfix · 2021-11-17T08:48:03Z

utils/db_helper.js

@@ -59,7 +74,7 @@ const GET_BET_INVESTORS =
  'SELECT buyer, direction, SUM(investmentamount) AS amount FROM amm_interactions WHERE bet = $1 GROUP BY buyer, direction;';

 const UPDATE_BALANCE_OF_USER =
-  'INSERT INTO token_balances (owner, symbol, last_update, balance) VALUES($1, $2, $3, $4) ON CONFLICT (owner, symbol) DO UPDATE SET last_update = $3, balance = token_balances.balance + $4 RETURNING balance;';
+  'UPDATE account SET balance = balance + $3 WHERE owner_account = $1 AND symbol = $2 AND account_namespace = $4 RETURNING balance;';


ok now I understand why some accounts are created. you removed UPDATE ON CONFLICT so you are not able to create new accounts on the fly. is this how ORM works? does it check if account exists and then creates it?

we need to create accounts on the fly + remove those spooky user acocunts

also we need to write to transaction log

It will fail in trading-engine if you try to mint or burn an account that does not exist. Are you suggesting we should change that logic and create accounts on mint and burn in case they do not exist?

Amir Zametica - External and others added 9 commits November 9, 2021 22:46

Use account namespaces

d2073b7

- bet noblock

ed5bebf

- fix cannot buy all outcome tokens

525836f

Adapt tests for sell amount

aee2ab7

- update amm and refund tests

6dee113

[fix] resolved conflicts with trunk branch

95e6bc9

[fix] corrected query statement

725869f

[fix] added missed method

c38802a

[fix] unit test issue

06e01bb

rudolfix commented Nov 17, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DO NOT MERGE] new accounts table #49

[DO NOT MERGE] new accounts table #49

rudolfix commented Nov 15, 2021

rudolfix left a comment

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

rudolfix Nov 17, 2021

zametica Nov 17, 2021

rudolfix Nov 17, 2021

zametica Nov 17, 2021

rudolfix Nov 17, 2021

zametica Nov 17, 2021

rudolfix Nov 17, 2021

zametica Nov 17, 2021

[DO NOT MERGE] new accounts table #49

Are you sure you want to change the base?

[DO NOT MERGE] new accounts table #49

Conversation

rudolfix commented Nov 15, 2021

rudolfix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment