forked from yacoin/yacoin
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MatJaz's new logic in TestNet only and .conf-urable and more #9
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. The call to TransactionRecord::showTransaction() in transactiontablemodel.cpp updateWallet() seems to ignore minted blocks after the "switch over" date, unless I'm misreading something? See comments I added in the code, and changes. The output looks the same and correct to me. 2. I still think we should use our own ports not bitcoins! So I changed the Qt help message string in bitcoinstrings.cpp on RPC ports. 3. I'm still trying to restore the NVC Qt code to differentiate between minted and mined coinbase transactions in transactionview.cpp, since all the mining code was removed beforehand! Perhaps others can complete the task. ATM, minted blocks are shown as mined. And there is no real mined icon. And the terminology is scrambled, since mining was abadoned, minting is called mined throughout the code, yuck! I have tried to change it to the correct terminology, where I have added or changed code. Code I haven't changed, or haven't seen, or haven't understood, I've left alone! Though it should be changed too! 4. I've added a new .conf item, a boolean 'testnetNewLogic' to trigger Matjaz's new logic, which for the moment, can only be enabled it TestNet mode. This can hold off the old implicit date time switch 'YACOIN_NEW_LOGIC_SWITCH_TIME' in timestamps.h I found it silly to keep changing it and recompiling, which would cause a release of new exes needlessly. Also, block times are not the same +/- seconds or whatever across the net. I had thought of the bitcoin idea of median time past, see 'GetMedianTimePast()' in main.h, but since it uses the undocumented, uncommented magic, contextually challenged naked number, identical to Bitcoin's code (of 11 for whatever it's worth) I thought that if that was meant to be a roughly 50 minute average block delay in bitcoins world, then it would have to be ~100 in Yacoin's world to also be ~50 minutes. But since (again!) there is no documentation, anywhere, I thought it would be better to just leave it to someone who knows(?), so I went the above route. Also there is a companion .conf item, a number 'testnetnewlogicblocknumber', which is read after 'LoadBlockIndex()' in init.cpp, and is set to the best block height 'pindexBest->nHeight' unless the .conf file sets it to something else. The code then immediately calls 'HaveWeSwitchedToNewLogicRules()' to decide whether or not to switch to new logic, or not. This way, we can turn on new logic testing, in TestNet at the moment, on any Testnet block we want, without recompiling. We can further, when we are happy with the new logic, change the guts of that function and turn the new logic on in MainNet too!! One change all in one place. 5. I have removed the global namespace polluting 'using namespace std;' from the code, and replaced it with appropriate using 'using std::string;', etc. phrases, which at least explicitly states which std::functions one is adding to that files global namespace. If you research this you wil find the preponderance of evidence is to not use it. 6. Added, for completeness, not mandatory, unless .cpp files are nested, etc., the end of file #ifdef _MSC_VER #include "msvc_warnings.pop.h" #endif to those files that should have had it, which is a lot! 7. In main.cpp and elsewhere, tried to replace all the tests for new logic with one boolean, 'fUsingOld044Rules' to make it clearer what we are doing, and where we have 'old' 0.4.4 and 'new' Matjaz's logic. There are places I'm not sure how things should go! E.g. lines 3264, 3389 on main.cpp The code seems to work in Testnet and MainNet so I don't know? Others should look at it with more 'context' than I can bring to the issue!! 8. Back to an old issue I asked about and got no response, the 'CWallet::GetStake()' in wallet.cpp, issue of how to display the stake of minted coins in the transaction display of Qt. It seems to have been wrong since the first NVC 0.4.5 and all following versions. I believe it has to do with the fact that the mining code was removed, the teminology wasn't changed, and we YAC-ers use both minting and mining and well you get the idea. :) You see it in the transaction display list in Qt. It always was wrong and didn't match the old 0.4.4 list, but since the code was changed so much, and is so Qt obscure and undocumented, (where have I heard that before [lol]) I leave to the Qt-ers with more time than I have, to fix it! 9. I put a comment, actually many in 'GetBoolArg()' in util.cpp since I found it to be such a poor rendition of 'get a boolean argument for this name'. Come on, has to be 0 if it exists! And crashes (!!!!) if it is just named with no = And returns true if unassigned, i.e "junk =". And return false on "junk = true" (rotflmao). Anyway, I put in some suggestions. I think that just because bitcoin still uses this (and pollutes the global namespace) after 7 years, doesn't mean we should follow along like sheep. We can do better. Think of how much clearer the .conf file can be if named boolean flags are actually set to true or false! And be backward (which it really is!) compatible with the junky code that is there now. End of sermon. 10. In script.cpp, line 1035 and elsewhere, I try to show that post increment is bad form in loop variables, especially if they are not simple data types, and even if they are!! Technically a compiler hint to not load the value anywhere, register, location, etc. 11. The code in netbase.cpp is more in line with the code in the latest bitcoin sources. 12. In price.cpp, line 305, I'm guessing how to do linux code, but I think it should work! Test and see if getyacprice works for you?
…ws builds changed getyacprice to work on non windows builds, but needs testing on non windows systems, Mac, etc. Tuned for better node capture and tested. Version displays as 0.4.8.3 in a releases so that it can be identified.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The call to TransactionRecord::showTransaction() in
transactiontablemodel.cpp
updateWallet() seems to ignore minted blocks after the "switch over"
date, unless
I'm misreading something? See comments I added in the code, and
changes.
The output looks the same and correct to me.
I still think we should use our own ports not bitcoins! So I changed
the Qt help
message string in bitcoinstrings.cpp on RPC ports.
I'm still trying to restore the NVC Qt code to differentiate between
minted and mined
coinbase transactions in transactionview.cpp, since all the mining code
was removed
beforehand! Perhaps others can complete the task. ATM, minted blocks
are shown
as mined. And there is no real mined icon. And the terminology is
scrambled,
since mining was abadoned, minting is called mined throughout the code,
yuck!
I have tried to change it to the correct terminology, where I have added
or changed code.
Code I haven't changed, or haven't seen, or haven't understood, I've
left alone!
Though it should be changed too!
I've added a new .conf item, a boolean 'testnetNewLogic' to trigger
Matjaz's new logic,
which for the moment, can only be enabled it TestNet mode. This can
hold off the
old implicit date time switch 'YACOIN_NEW_LOGIC_SWITCH_TIME' in
timestamps.h
I found it silly to keep changing it and recompiling, which would cause
a release of
new exes needlessly. Also, block times are not the same +/- seconds or
whatever
across the net.
I had thought of the bitcoin idea of median time past, see
'GetMedianTimePast()' in main.h,
but since it uses the undocumented, uncommented magic, contextually
challenged naked
number, identical to Bitcoin's code (of 11 for whatever it's worth) I
thought that if that
was meant to be a roughly 50 minute average block delay in bitcoins
world, then it
would have to be ~100 in Yacoin's world to also be ~50 minutes. But
since (again!) there
is no documentation, anywhere, I thought it would be better to just
leave it to
someone who knows(?), so I went the above route. Also there is a
companion .conf item,
a number 'testnetnewlogicblocknumber', which is read after
'LoadBlockIndex()' in init.cpp,
and is set to the best block height 'pindexBest->nHeight' unless the
.conf file sets it to
something else. The code then immediately calls
'HaveWeSwitchedToNewLogicRules()' to decide
whether or not to switch to new logic, or not. This way, we can turn on
new logic testing,
in TestNet at the moment, on any Testnet block we want, without
recompiling. We can further,
when we are happy with the new logic, change the guts of that function
and turn the new logic
on in MainNet too!! One change all in one place.
I have removed the global namespace polluting 'using namespace std;'
from the code, and
replaced it with appropriate using 'using std::string;', etc. phrases,
which at least
explicitly states which std::functions one is adding to that files
global namespace. If
you research this you wil find the preponderance of evidence is to not
use it.
Added, for completeness, not mandatory, unless .cpp files are
nested, etc., the end of file
#ifdef _MSC_VER
#include "msvc_warnings.pop.h"
#endif
to those files that should have had it, which is a lot!
In main.cpp and elsewhere, tried to replace all the tests for new
logic with one boolean,
'fUsingOld044Rules' to make it clearer what we are doing, and where we
have 'old' 0.4.4 and
'new' Matjaz's logic.
There are places I'm not sure how things should go! E.g. lines 3264,
3389 on main.cpp The
code seems to work in Testnet and MainNet so I don't know? Others
should look at it
with more 'context' than I can bring to the issue!!
'CWallet::GetStake()' in wallet.cpp,
issue of how to display the stake of minted coins in the transaction
display of Qt.
It seems to have been wrong since the first NVC 0.4.5 and all following
versions. I believe it
has to do with the fact that the mining code was removed, the teminology
wasn't changed, and
we YAC-ers use both minting and mining and well you get the idea. :)
You see it in the
transaction display list in Qt. It always was wrong and didn't match
the old 0.4.4 list, but
since the code was changed so much, and is so Qt obscure and
undocumented, (where have I heard
that before [lol]) I leave to the Qt-ers with more time than I have, to
fix it!
found it to be such a
poor rendition of 'get a boolean argument for this name'. Come on, has
to be 0 if it exists!
And crashes (!!!!) if it is just named with no = And returns true if
unassigned, i.e "junk =".
And return false on "junk = true" (rotflmao). Anyway, I put in some
suggestions. I think that
just because bitcoin still uses this (and pollutes the global namespace)
after 7 years, doesn't
mean we should follow along like sheep. We can do better.
Think of how much clearer the .conf file can be if named boolean flags
are actually set to true
or false! And be backward (which it really is!) compatible with the
junky code that is there now.
End of sermon.
In script.cpp, line 1035 and elsewhere, I try to show that post
increment is bad form in loop
variables, especially if they are not simple data types, and even if
they are!!
Technically a compiler hint to not load the value anywhere, register,
location, etc.
The code in netbase.cpp is more in line with the code in the latest
bitcoin sources.
In price.cpp, line 305, I'm guessing how to do linux code, but I
think it should work! Test
and see if getyacprice works for you?