75. it is 75 million. In only one of the accounts.
And if everyone who hold their Nxt in dgex tries to withdraw... Dgex will get 75M * 1.4%. |
|
|
|
Edit: I can't explain why, coz I can leak info about a flaw accidentally, sorry.
Let me guess, we need to add amount -= transaction.amount when transaction.senderPublicKey is equals to publicKey? I mean loop in Account.getEffectiveBalance() method. It can be wrong assumption because I still doesn't understand well what is effective balance  |
|
|
|
I didn't know that, are you sure? From the java source of ConcurrentHashMap, which Nxt.peers already is in 0.4.9e:
You are right, values backed by the map. Hmm, I remember the times when values() return copy of map. Or my memory is very bad... |
|
|
|
Just make sure you don't give him/them the whole 0.4.9e code in this topic!  I have no time to be full-time Nxt core developer  |
|
|
|
6821 lines java class?. THAT is more than a fatal flaw. Looking at the code is giving me a headache.  It's not a problem with any modern IDE. Just use code folding. |
|
|
|
I hope you already remove "double copy" peers = ((HashMap<String, Peer>)Nxt.peers.clone()).values(); from thread, which is start every second |
|
|
|
My replacement of the same lines in getAnyPeer, already in 0.4.9e: List<Peer> selectedPeers = new ArrayList<Peer>();
for (Peer peer : Nxt.peers.values()) {
if (peer.blacklistingTime <= 0 && peer.state == state && peer.announcedAddress.length() > 0
&& (!applyPullThreshold || !enableHallmarkProtection || peer.getWeight() >= pullThreshold)) {
selectedPeers.add(peer);
}
}
AFAIR, HashMap.values() make a copy, while HashMap.entrySet() definitely not. So my variant is even better. You sync in peers anyway, so it is safe. |
|
|
|
This proves u r not BCNext. Ur and his logic r completely different!
I am from generation "640k is enough" So I dislike such memory copies. |
|
|
|
Keep digging, I am still a step ahead. Two step ahead (0.4.8 and 0.4.9), and I hope you start to do third step already |
|
|
|
Three is only one file which is a 6812 lies Nxt.java.
It is not a news for those who tries to decompile it since first version |
|
|
|
Relax, I am also using IntelliJ. And the above has been fixed in 0.4.9e.
I have fixed, I hope, the synchronization issues related to Blocks and Transactions in 0.4.9e. The synchronization in Accounts is on my TODO list for the next version.
Good news! Okey, let me find something really interesting... |
|
|
|
Okey, let me rewrite something static Peer getAnyPeer(int state, boolean applyPullThreshold) {
synchronized (peers) {
Collection<Peer> peers = ((HashMap<String, Peer>)Nxt.peers.clone()).values();
Iterator<Peer> iterator = peers.iterator();
while (iterator.hasNext()) {
Peer peer = iterator.next();
if (peer.blacklistingTime > 0 || peer.state != state || peer.announcedAddress.length() == 0 || (applyPullThreshold && enableHallmarkProtection && peer.getWeight() < pullThreshold)) {
iterator.remove();
}
}
if (peers.size() > 0) {
Peer[] selectedPeers = peers.toArray(new Peer[0]);
...
For memory usage I prefer to code like this: List<Peer> selectedPeers = new ArrayList<>();
for (Map.Entry<String, Peer> entry : peers.entrySet()) {
Peer peer = entry.getValue();
if (peer.blacklistingTime <= 0 && peer.state == state && peer.announcedAddress.length() != 0 && ((!applyPullThreshold || !enableHallmarkProtection || peer.getWeight() >= pullThreshold))) {
selectedPeers.add(peer);
}
} And even more, List selectedPeers can be reused, if used only in this method. |
|
|
|
|
One more thanks to IDEA: line 6533, peer can be null. At least there's check for null above and below, but not here.
|
|
|
|
Small suggestion on usage of Collection.toArray(). I see a lot of calls like peers.toArray(new Peer[0]) 1. It is completely useless and equal to simple toArray() call. 2. It is better in terms of speed and memory usage to code like this: peers.toArray(new Peer[peers.size()]) |
|
|
|
Do you accept any suggestions here, or flaws only?
What if I found some potentially bad code?
Only flaws, feel free to create another thread for suggestions and post link here. . Feel free everyone to post you suggestions there. |
|
|
|
While lurking for injected bugs, I found that some parts of Nxt code makes my eyes bleed. With hope that I have enough programming skills (probably better, than my runglish ), I'm starting this thread. Not for bounty, but for better quality (but donations welcome, as usual ). First of all, thanks to IntelliJ IDEA code analysis, I see a lot of syncronized with potentially wrong usage. Static variables like peers, blocks, accounts, users are not final. They are initialized at start, so it is not a big flaw right now, but who knows about future. When I look more deeply in syncronizations, I see a lot of places, where author get content of such global collection and then sync on it, but not sync on collection itself. Example: Block.analyse() method:
Account generatorAccount = accounts.get(Account.getId(generatorPublicKey));
synchronized (generatorAccount) {
generatorAccount.setBalance(generatorAccount.balance + totalFee * 100L);
generatorAccount.setUnconfirmedBalance(generatorAccount.unconfirmedBalance + totalFee * 100L);
} While in some places such usage may be safe, I think it's very bad practice anyway. More to come... |
|
|
|
|
Do you accept any suggestions here, or flaws only?
What if I found some potentially bad code?
|
|
|
|
Hey devs, I need thread where I will start to complain to sources?  Not for bounty, but for pleasure. |
|
|
|
I did my two week audition, I guess I failed. Maybe you're coming too early with your ideas. |
|
|
|
Unfortunately due to the lack of support from the founders, with the exception of aldrin, I am forced to say goodbye. James, you definitely need to start separate thread with your ideas. This thread is a flood of everything about Nxt, from deep techinfo to asking for donations. |
|
|
|
|
Show Posts
