From 7f7cdf753d7ace2714d560121a8dbfc62c0568d9 Mon Sep 17 00:00:00 2001 From: Luke Imhoff Date: Wed, 24 Oct 2018 12:55:40 -0500 Subject: [PATCH] Switch token_transfers to composite primary key * Drop id. * Make (transaaction_hash, log_index) the primary key. This ports the change from `internal_transactions`. --- apps/explorer/lib/explorer/chain.ex | 2 +- .../lib/explorer/chain/token_transfer.ex | 12 +++++-- ..._token_transfers_composite_primary_key.exs | 22 ++++++++++++ .../explorer/chain/token_transfer_test.exs | 15 ++++---- apps/explorer/test/explorer/chain_test.exs | 36 +++++++++++++------ 5 files changed, 67 insertions(+), 20 deletions(-) create mode 100644 apps/explorer/priv/repo/migrations/20181024172010_token_transfers_composite_primary_key.exs diff --git a/apps/explorer/lib/explorer/chain.ex b/apps/explorer/lib/explorer/chain.ex index ca1fad5984..9be85c9c6f 100644 --- a/apps/explorer/lib/explorer/chain.ex +++ b/apps/explorer/lib/explorer/chain.ex @@ -1967,7 +1967,7 @@ defmodule Explorer.Chain do left_join: tf in TokenTransfer, on: tf.transaction_hash == l.transaction_hash and tf.log_index == l.index, where: l.first_topic == unquote(TokenTransfer.constant()), - where: is_nil(tf.id), + where: is_nil(tf.transaction_hash) and is_nil(tf.log_index), select: t.block_number, distinct: t.block_number ) diff --git a/apps/explorer/lib/explorer/chain/token_transfer.ex b/apps/explorer/lib/explorer/chain/token_transfer.ex index 3e5ae71b4e..1d454e1809 100644 --- a/apps/explorer/lib/explorer/chain/token_transfer.ex +++ b/apps/explorer/lib/explorer/chain/token_transfer.ex @@ -62,9 +62,10 @@ defmodule Explorer.Chain.TokenTransfer do @constant "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef" + @primary_key false schema "token_transfers" do field(:amount, :decimal) - field(:log_index, :integer) + field(:log_index, :integer, primary_key: true) field(:token_id, :decimal) belongs_to(:from_address, Address, foreign_key: :from_address_hash, references: :hash, type: Hash.Address) @@ -78,7 +79,12 @@ defmodule Explorer.Chain.TokenTransfer do type: Hash.Address ) - belongs_to(:transaction, Transaction, foreign_key: :transaction_hash, references: :hash, type: Hash.Full) + belongs_to(:transaction, Transaction, + foreign_key: :transaction_hash, + primary_key: true, + references: :hash, + type: Hash.Full + ) has_one(:token, through: [:token_contract_address, :token]) @@ -195,7 +201,7 @@ defmodule Explorer.Chain.TokenTransfer do tt in TokenTransfer, join: t in Token, on: tt.token_contract_address_hash == t.contract_address_hash, - select: {tt.token_contract_address_hash, count(tt.id)}, + select: {tt.token_contract_address_hash, fragment("COUNT(*)")}, group_by: tt.token_contract_address_hash ) diff --git a/apps/explorer/priv/repo/migrations/20181024172010_token_transfers_composite_primary_key.exs b/apps/explorer/priv/repo/migrations/20181024172010_token_transfers_composite_primary_key.exs new file mode 100644 index 0000000000..a973647898 --- /dev/null +++ b/apps/explorer/priv/repo/migrations/20181024172010_token_transfers_composite_primary_key.exs @@ -0,0 +1,22 @@ +defmodule Explorer.Repo.Migrations.TokenTransfersCompositePrimaryKey do + use Ecto.Migration + + def up do + # Remove old id + alter table(:token_transfers) do + remove(:id) + end + + # Don't use `modify` as it requires restating the whole column description + execute("ALTER TABLE token_transfers ADD PRIMARY KEY (transaction_hash, log_index)") + end + + def down do + execute("ALTER TABLE token_transfers DROP CONSTRAINT token_transfers_pkey") + + # Add back old id + alter table(:token_transfers) do + add(:id, :bigserial, primary_key: true) + end + end +end diff --git a/apps/explorer/test/explorer/chain/token_transfer_test.exs b/apps/explorer/test/explorer/chain/token_transfer_test.exs index 040884afd8..a48326cc8c 100644 --- a/apps/explorer/test/explorer/chain/token_transfer_test.exs +++ b/apps/explorer/test/explorer/chain/token_transfer_test.exs @@ -50,12 +50,15 @@ defmodule Explorer.Chain.TokenTransferTest do token: token ) - transfers_ids = + transfers_primary_keys = token_contract_address.hash |> TokenTransfer.fetch_token_transfers_from_token_hash([]) - |> Enum.map(& &1.id) + |> Enum.map(&{&1.transaction_hash, &1.log_index}) - assert transfers_ids == [another_transfer.id, token_transfer.id] + assert transfers_primary_keys == [ + {another_transfer.transaction_hash, another_transfer.log_index}, + {token_transfer.transaction_hash, token_transfer.log_index} + ] end test "when there isn't token transfers won't show anything" do @@ -101,14 +104,14 @@ defmodule Explorer.Chain.TokenTransferTest do paging_options = %PagingOptions{key: first_page.inserted_at, page_size: 1} - token_transfers_ids_paginated = + token_transfers_primary_keys_paginated = TokenTransfer.fetch_token_transfers_from_token_hash( token_contract_address.hash, paging_options: paging_options ) - |> Enum.map(& &1.id) + |> Enum.map(&{&1.transaction_hash, &1.log_index}) - assert token_transfers_ids_paginated == [second_page.id] + assert token_transfers_primary_keys_paginated == [{second_page.transaction_hash, second_page.log_index}] end end diff --git a/apps/explorer/test/explorer/chain_test.exs b/apps/explorer/test/explorer/chain_test.exs index a2ff28c95c..21ddaa3136 100644 --- a/apps/explorer/test/explorer/chain_test.exs +++ b/apps/explorer/test/explorer/chain_test.exs @@ -235,7 +235,10 @@ defmodule Explorer.ChainTest do insert(:token_transfer, to_address: build(:address), transaction: transaction) transaction = Chain.address_to_transactions(address) |> List.first() - assert transaction.token_transfers |> Enum.map(& &1.id) == [token_transfer.id] + + assert transaction.token_transfers |> Enum.map(&{&1.transaction_hash, &1.log_index}) == [ + {token_transfer.transaction_hash, token_transfer.log_index} + ] end test "returns just the token transfers related to the given contract address" do @@ -250,7 +253,10 @@ defmodule Explorer.ChainTest do insert(:token_transfer, to_address: build(:address), transaction: transaction) transaction = Chain.address_to_transactions(contract_address) |> List.first() - assert Enum.map(transaction.token_transfers, & &1.id) == [token_transfer.id] + + assert Enum.map(transaction.token_transfers, &{&1.transaction_hash, &1.log_index}) == [ + {token_transfer.transaction_hash, token_transfer.log_index} + ] end test "returns all token transfers when the given address is the token contract address" do @@ -572,12 +578,17 @@ defmodule Explorer.ChainTest do |> insert() |> with_block() - %TokenTransfer{id: token_transfer_id, token_contract_address_hash: token_contract_address_hash} = - insert(:token_transfer, to_address: address, transaction: transaction) + %TokenTransfer{ + transaction_hash: token_transfer_transaction_hash, + log_index: token_transfer_log_index, + token_contract_address_hash: token_contract_address_hash + } = insert(:token_transfer, to_address: address, transaction: transaction) assert token_contract_address_hash |> Chain.fetch_token_transfers_from_token_hash() - |> Enum.map(& &1.id) == [token_transfer_id] + |> Enum.map(&{&1.transaction_hash, &1.log_index}) == [ + {token_transfer_transaction_hash, token_transfer_log_index} + ] end end @@ -778,7 +789,7 @@ defmodule Explorer.ChainTest do |> insert_list(:transaction) |> with_block() - %TokenTransfer{id: id1} = + %TokenTransfer{transaction_hash: transaction_hash1, log_index: log_index1} = insert( :token_transfer, to_address: address, @@ -787,7 +798,7 @@ defmodule Explorer.ChainTest do token: token ) - %TokenTransfer{id: id2} = + %TokenTransfer{transaction_hash: transaction_hash2, log_index: log_index2} = insert( :token_transfer, to_address: address, @@ -799,7 +810,10 @@ defmodule Explorer.ChainTest do fetched_transactions = Explorer.Chain.hashes_to_transactions([transaction1.hash, transaction2.hash]) assert Enum.all?(fetched_transactions, fn transaction -> - hd(transaction.token_transfers).id in [id1, id2] + %TokenTransfer{transaction_hash: transaction_hash, log_index: log_index} = + hd(transaction.token_transfers) + + {transaction_hash, log_index} in [{transaction_hash1, log_index1}, {transaction_hash2, log_index2}] end) end end @@ -2004,9 +2018,11 @@ defmodule Explorer.ChainTest do |> insert() |> with_block() - %TokenTransfer{id: id} = insert(:token_transfer, transaction: transaction) + %TokenTransfer{transaction_hash: transaction_hash, log_index: log_index} = + insert(:token_transfer, transaction: transaction) - assert [%TokenTransfer{id: ^id}] = Chain.transaction_to_token_transfers(transaction) + assert [%TokenTransfer{transaction_hash: ^transaction_hash, log_index: ^log_index}] = + Chain.transaction_to_token_transfers(transaction) end test "token transfers necessity_by_association loads associations" do