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

Full node support for Dash #2424

Merged
merged 6 commits into from
Aug 16, 2023
Merged

Full node support for Dash #2424

merged 6 commits into from
Aug 16, 2023

Conversation

dev-warrior777
Copy link
Contributor

@dev-warrior777 dev-warrior777 commented Jul 4, 2023

Full node support for Dash

original pr

@dev-warrior777 dev-warrior777 marked this pull request as ready for review July 4, 2023 11:16
@JoeGruffins
Copy link
Member

Why the new pr?

@dev-warrior777
Copy link
Contributor Author

dev-warrior777 commented Jul 5, 2023

Why the new pr?

My fork github repo was messed up ..

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

I'm still not sure about the Firo ports change, but everything else looks good to me.

I still see the error, when applying #2425 [ERR] CORE[dash]: error unlocking spent coins: rawrequest (lockunspent) error: -8: Invalid parameter, expected unspent output

It looks to me like the output is already spent from the point of view of the node before we call unlock. While we should figure out what's going on there, it doesn't look like a real problem to me, just a difference between bitcoind and dashd. The coins should be spent by that time by the split tx, but bitcoind does not remove them from the locked list automatically while dashd does maybe? Not sure.

@dev-warrior777
Copy link
Contributor Author

I'm still not sure about the Firo ports change,

Firo RPC Ports

8888 .. Firo is Asian and 8888 is prosperity ;-)

@dev-warrior777
Copy link
Contributor Author

dev-warrior777 commented Jul 5, 2023

I still see the error, when applying #2425 [ERR] CORE[dash]: error unlocking spent coins: rawrequest (lockunspent) error: -8: Invalid parameter, expected unspent output

I went into this deeper: In some cases I do actually see 'wrong' coins returned by the dex logic occasionally. Meaning they are not in the listlockunspent list as far as Dash is concerned when returned. This is the Dash RPC error. Since this was with debugger breakpoint not easy to tell if something not locked and a race was lost

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. I'm not sure how this file ended up here. You can drop this file and its containing directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

This is a side effect of this pr and directly running Dex code that asks for an address from inside the git code tree. In this case client/asset/dash/regnet_test.go.

I usually remember to clean up and did put in a pr to hide these directories from git but the granularity of the .gitignore is kinda unknown .. is it any code? I certainly see with other client asset tests.

Perhaps we should at least .gitignore any regtest/ directory above a certain height in the code tree. But what should be that height is not obvious.

Copy link
Member

Choose a reason for hiding this comment

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

This is a side effect of this pr and directly running Dex code that asks for an address from inside the git code tree. In this case client/asset/dash/regnet_test.go.

That file doesn't seem to exist. You can give steps to reproduce these misplaced files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sequence

  • Start Firo harness
cd dex/testing/firo
#export PATH to firod/firo-cli 
./harness.sh
  • run asset tests
cd client/asset/firo
# delete any regtest dir if found
go test -v -count=1 -tags=harness ./...

You should see a new directory regtest if the livetest code was successful

Copy link
Contributor Author

Choose a reason for hiding this comment

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

dev@dev-ThinkPad-T430 ~/dex/dcrdex/client/asset/firo (harness_mining)$ ls
firo.go  regnet_test.go
dev@dev-ThinkPad-T430 ~/dex/dcrdex/client/asset/firo (harness_mining)$ go test -v -count=1 -tags=harness ./...
=== RUN   TestWallet
    livetest.go:197: Setting up alpha/beta/gamma wallet backends...
2023-07-21 00:56:01.266 [DBG] TEST[alpha.alpha][RPC]: Node at 127.0.0.1:53768 reports subversion "/Satoshi:0.14.12.1/"
2023-07-21 00:56:01.302 [INF] TEST[alpha.alpha]: Connected wallet with current best block 3dc04a8987a951ae30bf7ce89653ae873ef1fd4d12b219b7160080b39bbd7d1d (352)
peer count = 3, err = <nil>2023-07-21 00:56:01.303 [DBG] TEST[beta.beta][RPC]: Node at 127.0.0.1:53769 reports subversion "/Satoshi:0.14.12.1/"

. . .

rate = 100, actual fee rate = 100 (22500 for 225 bytes), change = true
2023-07-21 00:56:04.972 [INF] TEST: Withdrew with a9a9f48aa475e4482be4e34919eb8370b57915b2f27940eb69a63550314ad421:0
--- PASS: TestWallet (3.71s)
=== RUN   TestFetchExternalFee
#### External fee rate fetched:: 1 sat/B
--- PASS: TestFetchExternalFee (0.91s)
PASS
ok  	decred.org/dcrdex/client/asset/firo	4.632s
dev@dev-ThinkPad-T430 ~/dex/dcrdex/client/asset/firo (harness_mining)$ ls
firo.go  regnet_test.go  regtest
dev@dev-ThinkPad-T430 ~/dex/dcrdex/client/asset/firo (harness_mining)$ 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@chappjc
Copy link
Member

chappjc commented Jul 21, 2023

Remember not to sync with master by merging into your feature branch. Please rebase. You may squash away all but one commit at this point, and that might be necessary to eliminate the merge commit.

dex/networks/dash/params.go Show resolved Hide resolved
dex/testing/dash/harness.sh Outdated Show resolved Hide resolved
@buck54321
Copy link
Member

Just needs a rebase and a cache buster update.

@buck54321 buck54321 merged commit aae4348 into decred:master Aug 16, 2023
5 checks passed
@dev-warrior777 dev-warrior777 deleted the dash branch August 16, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants