omgnetwork / omg-childchain-v2

pronounced /Ch-ch/
Apache License 2.0
5 stars 2 forks source link

performance of calculating fees from a block has to be improved #156

Closed pgebal closed 3 years ago

pgebal commented 3 years ago

Calculating fees for a full block from inputs and outputs of transactions included in the block is non-performant. If we calculate the fees on full block during finalization, using FeeClaim.generate_fee_transactions, https://github.com/omgnetwork/childchain/blob/ca40f3f18cf8b4b4718c3bdfff16e5fb7ecbbf32/apps/engine/lib/engine/fee/fee_claim.ex#L35 it takes over 15 s. to calculate fees.

Proposed solution is to store fees for a transaction on Transaction.insert. We calculate fees anyway, when validating a transaction.

Fees would be kept in db table fees with the following columns:

we'll have an unique index on (tx_hash, currency)

Then on finalizing block we can get all the fees by executing: SELECT currency, SUM(amount) FROM fees WHERE blknum = blknum GROUP_BY currency

Alternative solution would be to have (blknum, currency, amount) columns and updating amount on each transaction insert. It would work as long as we lock on inserting a transaction, but it introduces concurrency issues if we decide to insert without locking.

InoMurko commented 3 years ago

are there any foreign keys between fees and transaction?

pgebal commented 3 years ago

No. I don't see a use case for it. But they are in relation, so maybe we should have it. Not sure. @mederic-p what you think?

InoMurko commented 3 years ago

I think it makes sense to do this: fees table: tx_hash_fk, currency, amount

and you don't need anything else.

where tx_hash_fk is a foreign key from transactions.tx_hash

mederic-p commented 3 years ago

Any reason to use tx_hash over the id? Also, do we want the DB to be ready to support multiple fee token per transaction? I don't think it's needed for now, this can be updated later if needed.

pgebal commented 3 years ago

Any reason to use tx_hash over the id?

I can't see any. I think using id is good.

Also, do we want the DB to be ready to support multiple fee token per transaction? I don't think it's needed for now, this can be updated later if needed.

What do you mean? If that means fees in multiple currencies per transaction, then the schema proposed by Ino will do.