15Dkatz
Address Improper calculate balance bug
Context:
This addresses a Q&A post in the course where it was discovered that the validTransactionData was not marking transactions as invalid too eagerly. It turns out that there's a flaw in logic when it comes to calculating the balance. When validating a transaction, you need to calculate the balance of a wallet using only transaction outputs that existed before that transaction. Otherwise, the balance for a transaction calculation will improperly include transactions that were created after the transaction existed.
Notes:
- This approach leverages the timestamp property of the transaction input, and a new timestamp field within calculate balance. Adding a timestamp for
Wallet.calculateBalancewill only include transactions that were created before the timestamp. - This change is dirty, and untested. More bugs may follow.
- Not going to merge this into the master branch, in order to preserve master being as close to the course code as possible. But will leave this open for visibility.
Reference: https://www.udemy.com/instructor/communication/qa/8169222
Hi Dave when you make a payment to a non-existent account ie: panda then mine the transaction it doesn't seem to be shown as a rejected payment or credited back to your account.
From: 0497bb2cb17a7cdc6c09... | Your Qbit Account Balance: 200 To: foo... | Sent: 10 To: 0497bb2cb17a7cdc6c09... | Sent: 180 To: panda... | Sent: 10
Mined transaction complete
@scriptonian
Nope, didn't follow up and fix the tests after this change. As I noted in the description: "This change is dirty, and untested. More bugs may follow." If you're thinking about incorporating this fix into your project, I would definitely recommend fixing up the tests as well.
@k7n2g
That's currently a feature of the system. There's no checking on whether or not the address is one that already exists within the blockchain.
Awesome. Thank you David...sure i will fix it. Thank you again. You are the best! Really appreciate you jumping on this so fast.
So David, i fixed the test case. But i think your original solution may still have something off. the first and second transactions work after mine transaction...but anything after that you get "The incoming has invalid data". I am looking into it as well. So far nothing.
Hi Scriptopnian i get ReferenceError: timestamp is not defined
186 |
187 | it('creates a transaction with the reward input', () => {
> 188 | expect(rewardTransaction, timestamp).toEqual(REWARD_INPUT)
| ^
189 |
in transaction.test.js tried a number of ways to get to work any ideas to help please Dave
@k7n2g Yes this is because the rewardTransaction and the reward input a now different because of the timestamp. The mining reward in the config.js file does not have a timestamp. And this needs to be part of the test
I updated the test to be
it("creates a transaction with the reward input", () => {
//set rewardTransaction timestamp on reward input
const MINING_REWARD_INPUT_WITH_STAMP = {
...MINING_REWARD_INPUT,
timestamp: rewardTransaction.input.timestamp
};
expect(rewardTransaction.input).toEqual(MINING_REWARD_INPUT_WITH_STAMP);
});
Let me know if that works for you. I am still trying to figure out why anything after the third transaction results in "The incoming has invalid data" even after Davids attempt fix. So far i think it might have something to do with counting down in the calculateBalance, where the function that calls it (validTransactionData) is counting up. Not 100% sure yet.
Hi i now get } MINING_REWARD_INPUT is not defined
189 | //set rewardTransaction timestamp on reward input
190 | const MINING_REWARD_INPUT_WITH_STAMP = {
> 191 | ...MINING_REWARD_INPUT,
| ^
192 | timestamp: rewardTransaction.input.timestamp
193 | };
thats because my variable names are different. you called yours REWARD_INPUT , and i called mine MINING_REWARD_INPUT. So u have to change MINING_REWARD_INPUT to REWARD_INPUT :-)
hi seems to work now I added } const REWARD_INPUT = { timestamp: Date.now(), address: 'authorized-reward' }; to config five payments to foo and bar seem okay
Message received. Channel: BLOCKCHAIN. Message: [{"timestamp":1,"lastHash":"-----","hash":"hash-one","data":[],"nonce":6,"difficulty":6},{"timestamp":1568485695883,"lastHash":"hash-one","hash":"01f7357e109f10ba7459817149a4f6c857baba750cd522b75f418fd846612916","data":[{"id":"3b25d7b0-d71d-11e9-84a2-036494741380","outputMap":{"foo":10,"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":971,"bar":19},"input":{"timestamp":1568485679226,"amount":1000,"address":"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d","signature":{"r":"c0cc27e94b6cd1978aa81a4a6ac27040e9add0ab511189212325e71e2a1ee16e","s":"95eff50f8034bd5a6ebd9a41a1bdacb71d89a909e38686dd0ed6788e865e863a","recoveryParam":0}}},{"input":{"timestamp":1568485695874,"address":"authorized-reward"},"outputMap":{"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":50}}],"nonce":122,"difficulty":5},{"timestamp":1568485723202,"lastHash":"01f7357e109f10ba7459817149a4f6c857baba750cd522b75f418fd846612916","hash":"08420e8e094b92165af4ffbad49f5f256d59332185417b772c15acbc8fc8fd9a","data":[{"input":{"timestamp":1568485723202,"address":"authorized-reward"},"outputMap":{"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":50}}],"nonce":1,"difficulty":4},{"timestamp":1568485924202,"lastHash":"08420e8e094b92165af4ffbad49f5f256d59332185417b772c15acbc8fc8fd9a","hash":"1123e993d8caf07bf837344167417fe16f3c5311179656f36089342dc82b8af4","data":[{"id":"f01aea70-d71d-11e9-84a2-036494741380","outputMap":{"bar ":20,"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":1051},"input":{"timestamp":1568485919383,"amount":1071,"address":"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d","signature":{"r":"b59793a7fb5a339bd362ed61abd34537837c98702848a29c99199da1cd9f4dc5","s":"3cb5eac20cbc52d333a68d469e7d8cbcbaca2b58ff841cf2ae4f5ee9a78bba59","recoveryParam":0}}},{"input":{"timestamp":1568485924201,"address":"authorized-reward"},"outputMap":{"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":50}}],"nonce":2,"difficulty":3},{"timestamp":1568486003402,"lastHash":"1123e993d8caf07bf837344167417fe16f3c5311179656f36089342dc82b8af4","hash":"089e3bcabfa15b1565137e3bdf3c54d49fa81298e6239246c6c700038b0bb59d","data":[{"input":{"timestamp":1568486003402,"address":"authorized-reward"},"outputMap":{"0440d5155277fa9ba74c0599da24f8632415aa32c1687901e5b1dfff9a4b4442da75e33f3646d6dc54e6524bf8229fb5dd32999930c8ef400f5cf8b3086f04ed8d":50}}],"nonce":1,"difficulty":2}] The incoming chain must be longer
Glad to see your tests are passing now :-) Yes the error is NOT when you conduct transactions. The error is when you mine transactions and it occurs when u run npm start dev-peer. so you should have the peers running as well if you want to see it.
Hi David have updated all files all tests pass except this:-) wallet/transaction-pool.test.js ● Test suite failed to run
Your test suite must contain at least one test.
at node_modules/jest/node_modules/jest-cli/build/TestScheduler.js:256:22
have tried many tests but all fail can you help please Dave
hi david, when I run the start script with pubnub utilized, it tells me that the values in the pusub.pubnub constructor are undefined. Error log:
constructor({ blockchain, transactionPool, wallet }) { ^
TypeError: Cannot destructure property 'blockchain' of 'undefined' as it is undefined.
at new PubSub (C:\Users\Mickell\Desktop\Cypher-Network\app\pubsub.pubnub.js:16:17)
at Object.
I think that I have debug the problem. First we have to verify the balance for the incoming chain. The balance is update only when someone mining the transaction pool then the timestamp we need to calculate the theorize balance which have the previous block at the moment he was mining.
const trueBalance = Wallet.calculateBalance({
chain,
address: transaction.input.address,
timestamp: chain[i-1].timestamp
});
And the calculate balance algorithm is .... suspens ^_^ :
static calculateBalance ({ chain, address, timestamp }) {
let outputsTotal = 0
let hasConductedTransaction = false
let lessThanTimestamp = false
for (let i = chain.length - 1; i > 0; i--) {
lessThanTimestamp = chain[i].timestamp <= timestamp
for (const transaction of chain[i].data) {
if (transaction.input.timestamp >= timestamp && transaction.input.address !== REWARD_INPUT.address) {
break
}
if (transaction.input.address === address) {
hasConductedTransaction = true
}
const addressOutput = transaction.outputMap[address]
if (addressOutput) {
outputsTotal += addressOutput
}
}
if (hasConductedTransaction && lessThanTimestamp) break
}
return hasConductedTransaction
? outputsTotal
: STARTING_BALANCE + outputsTotal
}
I havent looked at this since september 2019, but i will verify in a few days. The last thing i said was "the first and second transactions work after mine transaction...but anything after that you get "The incoming has invalid data". @AdriwanKenoby did you confirm there wasn't anything invalid after the 3rd transaction?
I havent looked at this since september 2019, but i will verify in a few days. The last thing i said was "the first and second transactions work after mine transaction...but anything after that you get "The incoming has invalid data". @AdriwanKenoby did you confirm there wasn't anything invalid after the 3rd transaction?
Yes i confirm that i have saw the same problem and i fix it with the previous algorithm. The problem was improper calculate balance
I havent looked at this since september 2019, but i will verify in a few days. The last thing i said was "the first and second transactions work after mine transaction...but anything after that you get "The incoming has invalid data". @AdriwanKenoby did you confirm there wasn't anything invalid after the 3rd transaction?
Yes i confirm that i have saw the same problem and i fix it with the previous algorithm. The problem was improper calculate balance
Great! I was the one who originally reported this bug. Its been over a year and a half...BUT i will surely fire up the code again and apply the changes and see if its really fixed. Funny i was thinking recently about this project. Good job. i will get back to you!