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.

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

Fix tx expiring soon test #3784

Merged
merged 7 commits into from Jan 18, 2019

Conversation

@Eirik0
Copy link
Contributor

commented Jan 15, 2019

Closes #3718

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2019

This PR attempts to solve intermittent failures in the rpc test p2p_txexpiringsoon.py. Much of this PR is refactoring. There was a fair amount of duplicated code in this test and I cleaned it up as I was figuring out what was going on. I was able to reproduce the failure locally, but only about 1/20 times. I think the problem had to do with testnode2 not syncing properly. To mitigate this I added some extra asserting (for sanity) and some extra calls to sync_with_ping. The semantics of the test might be slightly different, but I think that the essence is in tact. I also connected node2 directly to node0 in case the issue was exacerbated by communicating via node1.

@Eirik0

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2019

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 15, 2019

⌛️ Trying commit 1e8fcaa with merge 6ec45fc...

zkbot added a commit that referenced this pull request Jan 15, 2019
@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 15, 2019

☀️ Test successful - pr-try
State: approved= try=True

@mms710 mms710 added this to Needs Prioritization in Arborist Team via automation Jan 15, 2019

@mms710 mms710 moved this from Needs Prioritization to Sprint Backlog in Arborist Team Jan 15, 2019

@mms710 mms710 moved this from Sprint Backlog to Sapling Priorites in Arborist Team Jan 15, 2019

@mms710 mms710 moved this from Sapling Priorites to PRs That Need Review + Their Associated Issues in Arborist Team Jan 15, 2019

@mdr0id mdr0id self-requested a review Jan 16, 2019

@mdr0id
mdr0id approved these changes Jan 17, 2019
Copy link
Contributor

left a comment

ACK

@mms710 mms710 moved this from PRs That Need Review + Their Associated Issues to Current Sprint in Arborist Team Jan 17, 2019

@str4d str4d added this to the v2.0.3 milestone Jan 17, 2019

@str4d
str4d approved these changes Jan 17, 2019
Copy link
Contributor

left a comment

utACK modulo comment

return received_pong
from test_framework.util import assert_equal, connect_nodes_bi, fail, \
initialize_chain_clean, p2p_port, start_nodes, sync_blocks, sync_mempools
from tx_expity_helper import TestNode, create_transaction

This comment has been minimized.

Copy link
@str4d

str4d Jan 17, 2019

Contributor

Typo in helper module name: tx_expiry_helper

This comment has been minimized.

Copy link
@Eirik0

Eirik0 Jan 17, 2019

Author Contributor

Whoops... Thanks for catching this. This is now fixed.

@Eirik0 Eirik0 force-pushed the Eirik0:3718-txexpiringsoon branch from 1e8fcaa to a482e4c Jan 17, 2019

@Eirik0 Eirik0 moved this from Current Sprint to In Review in Arborist Team Jan 17, 2019

@str4d

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2019

utACK

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2019

📌 Commit a482e4c has been approved by str4d

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2019

⌛️ Testing commit a482e4c with merge ac03c88...

zkbot added a commit that referenced this pull request Jan 18, 2019
@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 18, 2019

☀️ Test successful - pr-merge
Approved by: str4d
Pushing ac03c88 to master...

@zkbot zkbot merged commit a482e4c into zcash:master Jan 18, 2019

1 check passed

homu Test successful
Details

Arborist Team automation moved this from In Review to Released (Merged in Master) Jan 18, 2019

anthony19114 pushed a commit to CommerciumBlockchain/Commercium that referenced this pull request Feb 17, 2019
root
wallet: Skip transactions with no shielded data in CWallet:: SetBestCh
Use nullptr instead of NULL in wallet tests
Add Sapling test cases
zmq: add flag to publish all checked blocks
zmq: remove extraneous space from zmq_sub.py
Set Sprout note data in WalletTest.WriteWitnessCache
update libsodium dl-path ca333
Update zmq to 4.3.1
Move common code to helper
Make variables local
Check entire contents of mempool
fail test if pong is not received
Extract helper methods
Strategically sync to prevent intermittent failures
Return more information when building a transaction fails
throw an exception rather than returning false when building   invalid
merge of zcash#3784 - Eirik0:3718-txexpiringsoon, r=str4d
merge of zcash#3766 - ca333:patch-3, r=mdr0id
merge of zcash#3711 - str4d:wallet-atomic-write-optimisation,   r=
merge of zcash#3646 - Eirik0:transaction-builder-result, r=daira
zcash#3789 - rex4539:update-zmq, r=str4d
merge of zcash#3737 - gtank:zmq_checkedblock, r=str4d
Patch Proton for a minimal build.
Update nMinimumChainWork with information from the getblockchaininfo RPC
dependency-updates
bitzec added a commit to bitzec/bitzec that referenced this pull request Mar 24, 2019
update from zec 2.0.4 rc
Update ZMQ to 4.3.1 (zcash#3789)
Fix Tx expiring soon test (zcash#3784)
ZMQ: add flag to publish all checked blocks (zcash#3737)
wallet: Skip transactions with no shielded data in CWallet::SetBestChain() (zcash#3711)
Update z_mergetoaddress documentation (zcash#3699)
Allow user to ask server to save the Sprout R1CS to a file during startup (zcash#3691)
On shutdown, wait for miner threads to exit (join them) (zcash#3647)
Update for Mac OS local rpc-tests
Bitcoin 0.12 performance improvements (zcash#3263)
For a more complete list of changes, please see the 2.0.3 milestone.

For information on specific Sapling RPC parameter changes, please see the Network Upgrade Developer guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.