Skip to content
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.

Sign up for GitHub

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

Remove redundant checkAccept call #351

Closed
wants to merge 2 commits into from
Closed

Remove redundant checkAccept call #351

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0aeef22
Remove redundant checkAccept call
JoelKatz May 7, 2014
7c24d3f
[FOLD] add comment
JoelKatz May 7, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/ripple_app/ledger/LedgerMaster.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ class LedgerMasterImp
else
{
mLedgerHistory.builtLedger (newLCL);
checkAccept (newLCL);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my curiosity, why is this redundant? It isn't obvious, even reading the whole file...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new validation timing code is very smart about precisely when the last fully-validated ledger can be advanced. The call is here:
https://github.com/ripple/rippled/blob/check-accept/src/ripple_app/ledger/LedgerMaster.cpp#L784

}

Expand Down Expand Up @@ -770,6 +769,10 @@ class LedgerMasterImp
// Because we just built a ledger, we are no longer building one
setBuildingLedger (0);

// No need to process validations in standalone mode
if (getConfig().RUN_STANDALONE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And a comment here, perhaps, telling us why we can exit early?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can do that, but it really should be obvious. There are no validations in standalone mode. Who would you send them to? Who would you receive them from?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just say "There are no validations in standalone mode" - it wasn't totally
obvious to me that everything after that point was a validation.

On Wed, May 7, 2014 at 5:17 PM, JoelKatz notifications@github.com wrote:

In src/ripple_app/ledger/LedgerMaster.cpp:

@@ -770,6 +769,9 @@ class LedgerMasterImp
// Because we just built a ledger, we are no longer building one
setBuildingLedger (0);

  •    if (getConfig().RUN_STANDALONE)

I can do that, but it really should be obvious. There are no validations
in standalone mode. Who would you send them to? Who would you receive them
from?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/351/files#r12403406
.

return;

if (ledger->getLedgerSeq() <= mValidLedgerSeq)
{
WriteLog (lsINFO, LedgerConsensus)
Expand Down