-
Notifications
You must be signed in to change notification settings - Fork 104
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
harness: Update eth to work with geth 1.14 and up. #3123
base: master
Are you sure you want to change the base?
Conversation
401f927
to
bc85cd5
Compare
Does polygon need all the same changes? |
b5883f0
to
f8ce2cf
Compare
@@ -114,7 +114,7 @@ tmux send-keys -t $SESSION:0 "${DAEMON} -rpcuser=user -rpcpassword=pass \ | |||
-rpcport=${ALPHA_RPC_PORT} -datadir=${ALPHA_DIR} $(deprecateddbd $ALPHA_DESCRIPTOR_WALLET) \ | |||
-debug=rpc -debug=net -debug=mempool -debug=walletdb -debug=addrman -debug=mempoolrej \ | |||
-whitelist=127.0.0.0/8 -whitelist=::1 \ | |||
-txindex=1 -regtest=1 -port=${ALPHA_LISTEN_PORT} -fallbackfee=0.00001 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Bitcoin core v28.0.0 for some reason this wasn't using the port anymore. Using bind seems to work...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tried bitcoin core v22-1 and it accepts -bind=12345
but does not make a listening tcpip port.
2024-12-25T07:55:25Z Command-line arg: bind="12345"
2024-12-25T07:55:25Z Command-line arg: server="1"
netstat -tl <- no new port. i.e. it fails silently!
-port=12345
does make port:
tcp 0 0 0.0.0.0:12345 0.0.0.0:* LISTEN
tcp6 0 0 [::]:12345 [::]:* LISTEN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So -bind
does not work on earlier versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cannot say all earlier but I suspect so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did a quiick search on bitcoin and it looks a bit more complex ..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try just setting both...
I was going to update the internal eth nodes, but its adding a toolchain to the go.mod and we didn't want that for some reason. I guess we will have to add it to update though? |
f8ce2cf
to
726f473
Compare
The harness just mines whenever there is a transaction, which is unfortunate, but the mining scrip needs to be updated to reflect this. Also some tests in /client/asset/eth are broken still. |
726f473
to
b983ef2
Compare
Fixing the mining script https://github.com/decred/dcrdex/compare/726f473c18811544c6624b9239ddb78b555fe86f..b983ef294ec3571e685b9751863fc5de947ab172 I also tried setting |
e9173f2
to
4f832eb
Compare
Fixing tests and apparently gas for token swaps is less now https://github.com/decred/dcrdex/compare/b983ef294ec3571e685b9751863fc5de947ab172..4f832ebf1086d67112a0cae15b7c3a55c65032ae Also, I had to update to 1.14.12 because |
4f832eb
to
ed3f534
Compare
ed3f534
to
cdc734b
Compare
7469a1d
to
ef12c81
Compare
It looks like the newest version 1.14.13 does not require the toolchain thing to go.mod |
ef12c81
to
7588212
Compare
It looks like we must have go 1.22 though https://github.com/decred/dcrdex/compare/ef12c81a71930dd3b6bdf99fa997808390a0a738..758821247746b64667e366415276f05270ef6b9c |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried --dev.period
and it worked for me: b6ba4cd
I removed the mine-alpha script as well, since the mining is automatically handled by geth
. The only issue I had was that it would only mine one transaction per block, and I'm not sure why. The gas ceiling is higher than all of the pending transactions.
if info.MaxSwapsInTx != 20 { | ||
t.Fatalf("expected 20 for max swaps but got %d", info.MaxSwapsInTx) | ||
if info.MaxSwapsInTx != 24 { | ||
t.Fatalf("expected 24 for max swaps but got %d", info.MaxSwapsInTx) | ||
} | ||
if info.MaxRedeemsInTx != 45 { | ||
t.Fatalf("expected 45 for max redemptions but got %d", info.MaxRedeemsInTx) | ||
if info.MaxRedeemsInTx != 55 { | ||
t.Fatalf("expected 55 for max redemptions but got %d", info.MaxRedeemsInTx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did these change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The values are different with the changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heads up that #2038 removes these fields from WalletInfo
and we calculate them dynamically instead.
dex/testing/eth/harness.sh
Outdated
|
||
function setSenderBalance (tokenAddr, amt) { | ||
var testToken = contract.at(tokenAddr) | ||
return testToken.setSenderBalance(amt*1000000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in some contract the decimal will not be 6. You could check the decimals in the contract and multiply based on that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking out the conversion so the caller can do it. Hope thats good enough.
Yeah there was some reason the dev.period didnt work out, I think that's why. We can't work with one tx per block can we? |
a9b7a67
to
6c261a4
Compare
Reverted the test contract changes, fix most of martons review topics, also removing mining from the loadbot setup. The extra mining seems to be causing problems for geth and its not necessary in my opinion. https://github.com/decred/dcrdex/compare/a9b7a67d37daa752b7849579eb4c22e9aaf9ee22..6c261a4d1b727e0cfe0ba869a5814760eb600cdd Still using the mining script in the eth harness because we can't do with slow transactions. I guess it is mining one block per transaction though, now, so its a bit unnatural still. |
I think it's fine, generally we won't have more than 1 tx per block, and it's more realistic than immediately mining a block only when there is a transaction. I'm pretty sure we're doing something wrong though.. I don't think dev.period would have this restriction. I've messed with the gas ceiling and wasn't able to get it to work. |
The loadbot will produce many txn per second, and I use that alot. We can always adjust the scripts manually to meet our needs I suppose. I will look into if I am implementing this wrong. I haven't found anything yet about it only being 1 tx per block googling around. Unsure yet if that's by design. |
6c261a4
to
b86aacf
Compare
There`s some time dilation going on. The longer the dev node runs, the further the header's time drifts into the future. This means any contracts based off of local time will eventually be expired before created based on the chain's time. May be due to the fast blocks. Prints from where we watch blocks:
|
@@ -71,6 +72,7 @@ func runTrader(t Trader, name string) { | |||
} | |||
|
|||
maintain := true | |||
time.Sleep(time.Second * 16) // at least a block for reg funds |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to also listen for ctx.Done here
closes #3122
Also no longer using the deprecated
personal
namespace