Skip to content

Commit

Permalink
refactor: rename assetLockedAmount in CbTx to creditPoolBalance (d…
Browse files Browse the repository at this point in the history
…ashpay#5526)

## Issue being fixed or feature implemented
Bad naming is noticed in dashpay#5026 by
thephez

## What was done?
Renamed `assetLockedAmount` in CbTx to `creditPoolBalance`
Renamed also some local variables and functions to make it matched also.

## How Has This Been Tested?
Run functional/unit tests - succeed
Called python's rpc binding `node.getblock(block_hash)['cbTx']`:
Got this result:
```
{'version': 3, 'height': 1556, 'merkleRootMNList': '978b2b4d1b884de62799b9eaee75c7812fea59f98f80d5ff9c963b0f0f195e14', 'merkleRootQuorums': 'bc7a34eb114f4e4bf38a11080b5d8ac41bdb36dd41e17467bae23c94ba06b013', 'bestCLHeightDiff': 0, 'bestCLSignature': '000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000', 'creditPoolBalance': Decimal('7.00141421')}
```

## Breaking Changes
Renamed `assetLockedAmount` in CbTx to `creditPoolBalance`. @shumkov be
informed


## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
  • Loading branch information
knst committed Aug 24, 2023
1 parent 8f74f3a commit 78b37f6
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 52 deletions.
4 changes: 2 additions & 2 deletions src/evo/cbtx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,9 @@ bool CalcCbTxBestChainlock(const llmq::CChainLocksHandler& chainlock_handler, co

std::string CCbTx::ToString() const
{
return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, assetLockedAmount=%d.%08d)",
return strprintf("CCbTx(nVersion=%d, nHeight=%d, merkleRootMNList=%s, merkleRootQuorums=%s, bestCLHeightDiff=%d, bestCLSig=%s, creditPoolBalance=%d.%08d)",
nVersion, nHeight, merkleRootMNList.ToString(), merkleRootQuorums.ToString(), bestCLHeightDiff, bestCLSignature.ToString(),
assetLockedAmount / COIN, assetLockedAmount % COIN);
creditPoolBalance / COIN, creditPoolBalance % COIN);
}

std::optional<CCbTx> GetCoinbaseTx(const CBlockIndex* pindex)
Expand Down
6 changes: 3 additions & 3 deletions src/evo/cbtx.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class CCbTx
uint256 merkleRootQuorums;
uint32_t bestCLHeightDiff;
CBLSSignature bestCLSignature;
CAmount assetLockedAmount{0};
CAmount creditPoolBalance{0};

SERIALIZE_METHODS(CCbTx, obj)
{
Expand All @@ -48,7 +48,7 @@ class CCbTx
if (obj.nVersion >= CB_V20_VERSION) {
READWRITE(COMPACTSIZE(obj.bestCLHeightDiff));
READWRITE(obj.bestCLSignature);
READWRITE(obj.assetLockedAmount);
READWRITE(obj.creditPoolBalance);
}
}

Expand All @@ -68,7 +68,7 @@ class CCbTx
if (nVersion >= CB_V20_VERSION) {
obj.pushKV("bestCLHeightDiff", static_cast<int>(bestCLHeightDiff));
obj.pushKV("bestCLSignature", bestCLSignature.ToString());
obj.pushKV("assetLockedAmount", ValueFromAmount(assetLockedAmount));
obj.pushKV("creditPoolBalance", ValueFromAmount(creditPoolBalance));
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/evo/creditpool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo
if (!GetTxPayload(block->vtx[0]->vExtraPayload, cbTx)) {
throw std::runtime_error(strprintf("%s: failed-getcreditpool-cbtx-payload", __func__));
}
locked = cbTx.assetLockedAmount;
locked = cbTx.creditPoolBalance;
}

// We use here sliding window with LimitBlocksToTrace to determine
Expand Down Expand Up @@ -226,7 +226,7 @@ bool CCreditPoolDiff::SetTarget(const CTransaction& tx, TxValidationState& state
}

if (cbTx.nVersion == 3) {
targetLocked = cbTx.assetLockedAmount;
targetBalance = cbTx.creditPoolBalance;
}
return true;
}
Expand Down
8 changes: 4 additions & 4 deletions src/evo/creditpool.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class CCreditPoolDiff {
CAmount sessionUnlocked{0};

// target value is used to validate CbTx. If values mismatched, block is invalid
std::optional<CAmount> targetLocked;
std::optional<CAmount> targetBalance;

const CBlockIndex *pindex{nullptr};
public:
Expand All @@ -86,12 +86,12 @@ class CCreditPoolDiff {
return pool.locked + sessionLocked - sessionUnlocked;
}

const std::optional<CAmount>& GetTargetLocked() const {
return targetLocked;
const std::optional<CAmount>& GetTargetBalance() const {
return targetBalance;
}

std::string ToString() const {
return strprintf("CCreditPoolDiff(target=%lld, sessionLocked=%lld, sessionUnlocked=%lld, newIndexes=%lld, pool=%s)", GetTargetLocked() ? *GetTargetLocked() : -1, sessionLocked, sessionUnlocked, newIndexes.size(), pool.ToString());
return strprintf("CCreditPoolDiff(target=%lld, sessionLocked=%lld, sessionUnlocked=%lld, newIndexes=%lld, pool=%s)", GetTargetBalance() ? *GetTargetBalance() : -1, sessionLocked, sessionUnlocked, newIndexes.size(), pool.ToString());
}

private:
Expand Down
2 changes: 1 addition & 1 deletion src/evo/specialtxman.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, ll
}
if (creditPoolDiff != std::nullopt) {
CAmount locked_proposed{0};
if(creditPoolDiff->GetTargetLocked()) locked_proposed = *creditPoolDiff->GetTargetLocked();
if(creditPoolDiff->GetTargetBalance()) locked_proposed = *creditPoolDiff->GetTargetBalance();

CAmount locked_calculated = creditPoolDiff->GetTotalLocked();
if (locked_proposed != locked_calculated) {
Expand Down
2 changes: 1 addition & 1 deletion src/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
LogPrintf("CreateNewBlock() h[%d] CbTx failed to find best CL. Inserting null CL\n", nHeight);
}
assert(creditPoolDiff != std::nullopt);
cbTx.assetLockedAmount = creditPoolDiff->GetTotalLocked();
cbTx.creditPoolBalance = creditPoolDiff->GetTotalLocked();
}
}

Expand Down
78 changes: 39 additions & 39 deletions test/functional/feature_asset_locks.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,18 @@ def create_assetunlock(self, index, withdrawal, pubkey=None, fee=tiny_amount):
unlock_tx.vExtraPayload = unlockTx_payload.serialize()
return unlock_tx

def get_credit_pool_amount(self, node = None, block_hash = None):
def get_credit_pool_balance(self, node = None, block_hash = None):
if node is None:
node = self.nodes[0]

if block_hash is None:
block_hash = node.getbestblockhash()
block = node.getblock(block_hash)
return int(COIN * block['cbTx']['assetLockedAmount'])
return int(COIN * block['cbTx']['creditPoolBalance'])

def validate_credit_pool_amount(self, expected = None, block_hash = None):
def validate_credit_pool_balance(self, expected = None, block_hash = None):
for node in self.nodes:
locked = self.get_credit_pool_amount(node=node, block_hash=block_hash)
locked = self.get_credit_pool_balance(node=node, block_hash=block_hash)
if expected is None:
expected = locked
else:
Expand Down Expand Up @@ -241,38 +241,38 @@ def run_test(self):
asset_lock_tx = self.create_assetlock(coin, locked_1, pubkey)

self.check_mempool_result(tx=asset_lock_tx, result_expected={'allowed': True})
self.validate_credit_pool_amount(0)
self.validate_credit_pool_balance(0)
txid_in_block = self.send_tx(asset_lock_tx)
self.validate_credit_pool_amount(0)
self.validate_credit_pool_balance(0)
node.generate(1)
assert_equal(self.get_credit_pool_amount(node=node_wallet), 0)
assert_equal(self.get_credit_pool_amount(node=node), locked_1)
assert_equal(self.get_credit_pool_balance(node=node_wallet), 0)
assert_equal(self.get_credit_pool_balance(node=node), locked_1)
self.log.info("Generate a number of blocks to ensure this is the longest chain for later in the test when we reconsiderblock")
node.generate(12)
self.sync_all()

self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)

# tx is mined, let's get blockhash
self.log.info("Invalidate block with asset lock tx...")
block_hash_1 = node_wallet.gettransaction(txid_in_block)['blockhash']
for inode in self.nodes:
inode.invalidateblock(block_hash_1)
assert_equal(self.get_credit_pool_amount(node=inode), 0)
assert_equal(self.get_credit_pool_balance(node=inode), 0)
node.generate(3)
self.sync_all()
self.validate_credit_pool_amount(0)
self.validate_credit_pool_balance(0)
self.log.info("Resubmit asset lock tx to new chain...")
# NEW tx appears
asset_lock_tx_2 = self.create_assetlock(coin, locked_2, pubkey)
txid_in_block = self.send_tx(asset_lock_tx_2)
node.generate(1)
self.sync_all()
self.validate_credit_pool_amount(locked_2)
self.validate_credit_pool_balance(locked_2)
self.log.info("Reconsider old blocks...")
for inode in self.nodes:
inode.reconsiderblock(block_hash_1)
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)
self.sync_all()

self.log.info('Mine block with incorrect credit-pool value...')
Expand All @@ -281,7 +281,7 @@ def run_test(self):

self.log.info("Mine a quorum...")
self.mine_quorum()
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)

self.log.info("Testing asset unlock...")
asset_unlock_tx_index_too_far = self.create_assetunlock(10001, COIN, pubkey)
Expand All @@ -294,7 +294,7 @@ def run_test(self):
self.ensure_tx_is_not_mined(tx_too_far_index)

self.log.info("Generating several txes by same quorum....")
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)
asset_unlock_tx = self.create_assetunlock(101, COIN, pubkey)
asset_unlock_tx_late = self.create_assetunlock(102, COIN, pubkey)
asset_unlock_tx_too_late = self.create_assetunlock(103, COIN, pubkey)
Expand Down Expand Up @@ -322,11 +322,11 @@ def run_test(self):
self.send_tx(asset_unlock_tx)
self.mempool_size += 1
self.check_mempool_size()
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)
self.log.info("Mining one block - index '10001' can't be included in this block")
node.generate(1)
self.sync_all()
self.validate_credit_pool_amount(locked_1 - COIN)
self.validate_credit_pool_balance(locked_1 - COIN)
self.mempool_size -= 1
self.check_mempool_size()
self.log.info("Tx should not be mined yet... mine one more block")
Expand All @@ -347,20 +347,20 @@ def run_test(self):
reason = "double index")

self.log.info("Checking tx with too far index is mined too - it is not too far anymore...")
self.validate_credit_pool_amount(locked_1 - 2 * COIN)
self.validate_credit_pool_balance(locked_1 - 2 * COIN)
self.nodes[0].getrawtransaction(tx_too_far_index, 1)['blockhash']

self.log.info("Mining next quorum to check tx 'asset_unlock_tx_late' is still valid...")
self.mine_quorum()
self.log.info("Checking credit pool amount is same...")
self.validate_credit_pool_amount(locked_1 - 2 * COIN)
self.validate_credit_pool_balance(locked_1 - 2 * COIN)
self.check_mempool_result(tx=asset_unlock_tx_late, result_expected={'allowed': True})
self.log.info("Checking credit pool amount still is same...")
self.validate_credit_pool_amount(locked_1 - 2 * COIN)
self.validate_credit_pool_balance(locked_1 - 2 * COIN)
self.send_tx(asset_unlock_tx_late)
node.generate(1)
self.sync_all()
self.validate_credit_pool_amount(locked_1 - 3 * COIN)
self.validate_credit_pool_balance(locked_1 - 3 * COIN)

self.log.info("Generating many blocks to make quorum far behind (even still active)...")
self.slowly_generate_batch(too_late_height - node.getblock(node.getbestblockhash())["height"] - 1)
Expand All @@ -380,24 +380,24 @@ def run_test(self):
self.log.info("Test block invalidation with asset unlock tx...")
for inode in self.nodes:
inode.invalidateblock(block_asset_unlock)
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)
self.slowly_generate_batch(50)
self.validate_credit_pool_amount(locked_1)
self.validate_credit_pool_balance(locked_1)
for inode in self.nodes:
inode.reconsiderblock(block_to_reconsider)
self.validate_credit_pool_amount(locked_1 - 3 * COIN)
self.validate_credit_pool_balance(locked_1 - 3 * COIN)

self.log.info("Forcibly mining asset_unlock_tx_too_late and ensure block is invalid...")
self.create_and_check_block([asset_unlock_tx_too_late], expected_error = "bad-assetunlock-not-active-quorum")

node.generate(1)
self.sync_all()

self.validate_credit_pool_amount(locked_1 - 3 * COIN)
self.validate_credit_pool_amount(block_hash=block_hash_1, expected=locked_1)
self.validate_credit_pool_balance(locked_1 - 3 * COIN)
self.validate_credit_pool_balance(block_hash=block_hash_1, expected=locked_1)

self.log.info("Checking too big withdrawal... expected to not be mined")
asset_unlock_tx_full = self.create_assetunlock(201, 1 + self.get_credit_pool_amount(), pubkey)
asset_unlock_tx_full = self.create_assetunlock(201, 1 + self.get_credit_pool_balance(), pubkey)

self.log.info("Checking that transaction with exceeding amount accepted by mempool...")
# Mempool doesn't know about the size of the credit pool
Expand All @@ -413,7 +413,7 @@ def run_test(self):
self.create_and_check_block([asset_unlock_tx_full], expected_error = "failed-creditpool-unlock-too-much")

self.mempool_size += 1
asset_unlock_tx_full = self.create_assetunlock(301, self.get_credit_pool_amount(), pubkey)
asset_unlock_tx_full = self.create_assetunlock(301, self.get_credit_pool_balance(), pubkey)
self.check_mempool_result(tx=asset_unlock_tx_full, result_expected={'allowed': True })

txid_in_block = self.send_tx(asset_unlock_tx_full)
Expand All @@ -422,7 +422,7 @@ def run_test(self):
self.log.info("Check txid_in_block was mined...")
block = node.getblock(node.getbestblockhash())
assert txid_in_block in block['tx']
self.validate_credit_pool_amount(0)
self.validate_credit_pool_balance(0)

self.log.info("After many blocks duplicated tx still should not be mined")
self.send_tx(asset_unlock_tx_duplicate_index,
Expand All @@ -436,7 +436,7 @@ def run_test(self):
self.slowly_generate_batch(blocks_in_one_day + 1)
self.mine_quorum()

total = self.get_credit_pool_amount()
total = self.get_credit_pool_balance()
while total <= 10_900 * COIN:
self.log.info(f"Collecting coins in pool... Collected {total}/{10_900 * COIN}")
coin = coins.pop()
Expand All @@ -449,8 +449,8 @@ def run_test(self):
self.sync_mempools()
node.generate(1)
self.sync_all()
credit_pool_amount_1 = self.get_credit_pool_amount()
assert_greater_than(credit_pool_amount_1, 10_900 * COIN)
credit_pool_balance_1 = self.get_credit_pool_balance()
assert_greater_than(credit_pool_balance_1, 10_900 * COIN)
limit_amount_1 = 1000 * COIN
# take most of limit by one big tx for faster testing and
# create several tiny withdrawal with exactly 1 *invalid* / causes spend above limit tx
Expand All @@ -469,7 +469,7 @@ def run_test(self):
node.generate(1)
self.sync_all()

new_total = self.get_credit_pool_amount()
new_total = self.get_credit_pool_balance()
amount_actually_withdrawn = total - new_total
block = node.getblock(node.getbestblockhash())
self.log.info("Testing that we tried to withdraw more than we could...")
Expand All @@ -485,26 +485,26 @@ def run_test(self):
self.mempool_size = 1
self.check_mempool_size()

assert_equal(new_total, self.get_credit_pool_amount())
assert_equal(new_total, self.get_credit_pool_balance())
self.log.info("Fast forward to next day again...")
self.slowly_generate_batch(blocks_in_one_day - 2)
self.log.info("Checking mempool is empty now...")
self.mempool_size = 0
self.check_mempool_size()

self.log.info("Creating new asset-unlock tx. It should be mined exactly 1 block after")
credit_pool_amount_2 = self.get_credit_pool_amount()
limit_amount_2 = credit_pool_amount_2 // 10
credit_pool_balance_2 = self.get_credit_pool_balance()
limit_amount_2 = credit_pool_balance_2 // 10
index += 1
asset_unlock_tx = self.create_assetunlock(index, limit_amount_2, pubkey)
self.send_tx(asset_unlock_tx)
node.generate(1)
self.sync_all()
assert_equal(new_total, self.get_credit_pool_amount())
assert_equal(new_total, self.get_credit_pool_balance())
node.generate(1)
self.sync_all()
new_total -= limit_amount_2
assert_equal(new_total, self.get_credit_pool_amount())
assert_equal(new_total, self.get_credit_pool_balance())
self.log.info("Trying to withdraw more... expecting to fail")
index += 1
asset_unlock_tx = self.create_assetunlock(index, COIN * 100, pubkey)
Expand All @@ -515,7 +515,7 @@ def run_test(self):
self.log.info("generate many blocks to be sure that mempool is empty afterwards...")
self.slowly_generate_batch(60)
self.log.info("Checking that credit pool is not changed...")
assert_equal(new_total, self.get_credit_pool_amount())
assert_equal(new_total, self.get_credit_pool_balance())
self.check_mempool_size()

if __name__ == '__main__':
Expand Down

0 comments on commit 78b37f6

Please sign in to comment.