Nxt source code flaw reports

[gsan1]

Could you elaborate that? I've run a simulation over the hole blockchain (like every transaction did before is a game play), and the winning odds are 50/50. You could also write me a PM I am not sure if it is the right place to discuss that here.

Thank you CfB!  

ricot has just explained it.

Yes, i see the problem now. So it is better to wait for at least one confirmation right now. Anyway cool feature: Makes the assignment of bets to payments easier. But it would be really nice if it possible to improve that feature.

Maybe we could include T2 (that has T1 as ref transaction) only in a block if T1 is confirmed + the creator of the block, in which T1 got confirmed, is not equal to the sender or recipient of T2 or T1? So in the rare case when the recipient/sender of the block generates it, T2 waits for the next block. Could this solve the issue?

We are also developing some games for Nxt, and I was counting on referencedTransactions for instant bets.
How about if we make a rule in the game to invalidate (or delay, if with transparent mining we can predict who will forge next) every bet from the forger of the next block? That would make it safe?

In my opinion this is a problem that should by solved in the nxt code: ReferencedTransaction says that you can link a transcation to another and the second gets only confirmed when the first got. So we should ensure in the nxt code that this system is save.

[Come-from-Beyond]

That still wasn't any of the 3 announced bugs?

No.

It's useful for other applications.

can you give an example?

If u see that someone places a buy order, u can place another buy order a few NXT higher.

[Come-from-Beyond]

I really don't understand u or u don't understand me. The 2 chains (one with original block and the other with my corrupted block) have the same length and the same cumulative difficulty. It's just one transaction that is missing in one block. That doesn't change the length of the chain.

Next block will reference only one of ur blocks. Another will become orphaned.

First block chain:
.....<--> block i <--> block i+1 <--> block i+2 <--> ..... <--> block n = last Block

Second block chein:
.....<--> block i <--> corrupted block i+1 <--> block i+2 <--> ... <--> block n = last block

Same length, same difficulty, no way to distinguish. Clients will accept both chains and they don't even know that one transaction is missing.
BTW,  "Cunicula attack" and "Finney attack" are not related to that IMO.

2nd block will reference one of the previous blocks. The chains will be the same length only if other forger sends 2 twin blocks too. In this rare case we need one more block.

[Vega]

In my opinion this is a problem that should by solved in the nxt code: ReferencedTransaction says that you can link a transcation to another and the second gets only confirmed when the first got. So we should ensure in the nxt code that this system is save.

I agree, I meant as a workaround until then.
If we can just watch the unconfirmed transactions for bets this could be really fast.

[Come-from-Beyond]

We are also developing some games for Nxt, and I was counting on referencedTransactions for instant bets.
How about if we make a rule in the game to invalidate (or delay, if with transparent mining we can predict who will forge next) every bet from the forger of the next block? That would make it safe?

U (and the others) should wait when Instant Transactions r implemented. Then u don't need referenced transactions, even unconfirmed transactions will be confirmed with 100% guarantee.

[BloodyRookie]

I really don't understand u or u don't understand me. The 2 chains (one with original block and the other with my corrupted block) have the same length and the same cumulative difficulty. It's just one transaction that is missing in one block. That doesn't change the length of the chain.

Next block will reference only one of ur blocks. Another will become orphaned.

First block chain:
.....<--> block i <--> block i+1 <--> block i+2 <--> ..... <--> block n = last Block

Second block chein:
.....<--> block i <--> corrupted block i+1 <--> block i+2 <--> ... <--> block n = last block

Same length, same difficulty, no way to distinguish. Clients will accept both chains and they don't even know that one transaction is missing.
BTW,  "Cunicula attack" and "Finney attack" are not related to that IMO.

block i+2 contains a hash for block i+1, so if you change any bit in i+1, the original i+2 will not fit anymore...

No, version 0.4.7e does not contain the previous block hash. Thats exactly why it works.
And I could make you donwload my chain too by you telling that I got a chain with higher difficulty. Then you are interested in my chain and start downloading it. And in the end when your client compares the difficulty of your original chain with my corrupted chain (which has the very same difficulty)

Code:

if (Block.getLastBlock().cumulativeDifficulty.compareTo(curCumulativeDifficulty) < 0){...}

you keep my chain.

[ricot]

looking forward to that feature

@CfB: yes, setting a timeout for the whole blockchain-getting-thingy would solve the problem. also blacklist the peer, it's too slow anyways
but don't penalize him for fetching the whole blockchain from the beginning... that can take a while and it should ^^
(if you don't have an idea on how to do that, ask, i've got one in my head, just too lazy to type it out :p)

[Come-from-Beyond]

No, version 0.4.7e does not contain the previous block hash.

Block id is first 64bits of the block hash.

[ricot]

I really don't understand u or u don't understand me. The 2 chains (one with original block and the other with my corrupted block) have the same length and the same cumulative difficulty. It's just one transaction that is missing in one block. That doesn't change the length of the chain.

Next block will reference only one of ur blocks. Another will become orphaned.

First block chain:
.....<--> block i <--> block i+1 <--> block i+2 <--> ..... <--> block n = last Block

Second block chein:
.....<--> block i <--> corrupted block i+1 <--> block i+2 <--> ... <--> block n = last block

Same length, same difficulty, no way to distinguish. Clients will accept both chains and they don't even know that one transaction is missing.
BTW,  "Cunicula attack" and "Finney attack" are not related to that IMO.

block i+2 contains a hash for block i+1, so if you change any bit in i+1, the original i+2 will not fit anymore...

No, version 0.4.7e does not contain the previous block hash. Thats exactly why it works.
And I could make you donwload my chain too by you telling that I got a chain with higher difficulty. Then you are interested in my chain and start downloading it. And in the end when your client compares the difficulty of your original chain with my corrupted chain (which has the very same difficulty)

Code:

if (Block.getLastBlock().cumulativeDifficulty.compareTo(curCumulativeDifficulty) < 0){...}

you keep my chain.

block.generationSignature = Crypto.sign(Block.getLastBlock().generationSignature,
        secretPhrase);

[edit]
oh, wait...

[BloodyRookie]

Same length, same difficulty, no way to distinguish. Clients will accept both chains and they don't even know that one transaction is missing.
BTW,  "Cunicula attack" and "Finney attack" are not related to that IMO.

2nd block will reference one of the previous blocks. The chains will be the same length only if other forger sends 2 twin blocks too. In this rare case we need one more block.

I have the feeling I am talking against a wall
The corrupted block could be 100 block down the chain. A forger doesn't care about a block that was forged a long time ago. Anyway let's discuss that tomorrow.

[Come-from-Beyond]

looking forward to that feature

@CfB: yes, setting a timeout for the whole blockchain-getting-thingy would solve the problem. also blacklist the peer, it's too slow anyways
but don't penalize him for fetching the whole blockchain from the beginning... that can take a while and it should ^^
(if you don't have an idea on how to do that, ask, i've got one in my head, just too lazy to type it out :p)

I'll talk to Jean-Luc and PM u if we don't find a solution.

[Come-from-Beyond]

Anyway let's discuss that tomorrow.

Good idea, I'll go to sleep.

[gsan1]

We are also developing some games for Nxt, and I was counting on referencedTransactions for instant bets.
How about if we make a rule in the game to invalidate (or delay, if with transparent mining we can predict who will forge next) every bet from the forger of the next block? That would make it safe?

U (and the others) should wait when Instant Transactions r implemented. Then u don't need referenced transactions, even unconfirmed transactions will be confirmed with 100% guarantee.

This sounds cool. Can't wait for it.  
Good night!

[ricot]

ok, looked at it again:

the blockID is a hash over everything in the block (including a payloadHash).
so as soon as you change a transaction, your block will get a different id.
and the id of the previous block is also saved in a block, because that's the reference to building up the chain.

so: everything safe

[BloodyRookie]

The block Id argument is valid. It would have been easier if CfB would told that in his first answer

[okwolf]

Some of these are probably duplicates by now, but here's my 2 NXT:

• The entire source is implemented as a single file with lots of inner Classes. To enable distributed development this should be decomposed to at least one file per Class. This is especially true for Curve25519, which just added more clutter when merged into the same file.
• The genesis block is hardcoded into the source. Generally static data resources should be packaged separately as some sort of format that can be easily restored when requested.
• Why are hex strings used for serializing binary data to and from JSON over the wire? Other encodings such as Base64 would save on bandwidth and overhead of the protocol.
• What is the benefit of using unsigned longs for most of the ids? In particular the accounts. Although the hash is giving 256bits of output, 192 of them are being truncated and ignored in these cases.
• One barrier toward wider adoption of NXT is the requirement for whole numbers for amounts and fees. So far the code appears to have all of those values as ints or longs which would make transitioning to a divisible NXT difficult.

Lastly, have you guys thought about providing unit tests or using quality analysis tools like FindBugs, Checkstyle, or PMD? We're going through this at my day job right now, and I have to say early in the process is the best time to try set such standards.

Hi are you new to crypto or you just created a new account here ?

Not new to crypto, but new to NXT and this forum. Absolutely not new to Java development. Also have a pretty new account on nextcoin.org.

[Come-from-Beyond]

The block Id argument is valid. It would have been easier if CfB would told that in his first answer

I can't answer all questions. This was close to the flaw.

Oops...

[xibeijan]

smaragda, ricot, vamdor, ImmortAlex, rlh: Please PM your NXT tip account. Thanks for pouring through the code.

FrictionlessCoin send yours too cuz you need a NXT hug.

Post your addresses guys (gals?) I wanna reward your efforts...

ricot : 100K
ImmortAlex :50K
vamdor: 50K
rlh: 50K
smaragda: 50K

Well done guys!

[gimre]

In line 4594

Code:

if (Block.pushBlock(buffer, false)) {

checking the cumulativeDifficulty, if not satisfy, we load back blocks and transactions we just create at line 4637
but blocks.nxt.bak already have all the blocks in futureBlocks, so we will restore all incoming blocks(futureBlocks) along with all old blocks?

do I miss something, I just get stuck here. pls point it out. thx

These does something else than you think, this is way to *catch-up* with current block chain it works like this:
First:

• If possible, add block AFTER current last block
• otherwise save this block on futureBlocks list

So mostly stuff above will expand blockchain, but in case of fork, blocks will be added to futureBlocks

Now, if there are any blocks in futureBlocks:

• drop the TAIL of current blockchain as it is most likely BAD fork
• push blocks from futureBlocks to current chain
• Now best part if 'new' cumulativeDifficulty is smaller than old one, it means futureblocks we've just added are BAD
• so we load blockchain from disk, dropping all the changes, and adding peer to blacklist (as it seems he might have wanted to cheat us)
• Last save is done in case if last two steps didn't happen

Oh P.S, this can be done differently, and I'm actually doing it in a different way in my python code.

[Come-from-Beyond]

Oh P.S, this can be done differently, and I'm actually doing it in a different way in my python code.

Share, plz.