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

need to fix the test failure of the TestNonce method #24

Closed
dindinw opened this issue Dec 15, 2017 · 5 comments
Closed

need to fix the test failure of the TestNonce method #24

dindinw opened this issue Dec 15, 2017 · 5 comments

Comments

@dindinw
Copy link
Contributor

dindinw commented Dec 15, 2017

The TestNonce method keeps fail. it will call the HasValidNonce function to do the validation. it will check if the tx hash ends with 9s I think it only works for the p27 hashes like MRYSIXABICSX9XQSLPAPQHGAPCMBDQZXH9EOHPLL9LFQNUDTETNQFUJO9DPHTNPJI9BTQH9RM9I999999.

func TestNonce(t *testing.T) {
	var trytes Trytes = "QQ9VLGQTJICJH9UZNSBXRSKBWVXCTRNGLJPEYFSZBISDCGBHGAV9TEHIMJS9LMQNHFNWVAXALKGESPWZMUMZPQCKCUFSEJZVHWZHZOEZBPTERXPHUQLQLZMENKOKAWMJ9LCFSIBSBEYCJQVQQMTMRJMDEKRXLCIRZFWQBRJYLPSK9XLWFYFTU9FBJATWPQRJBHWYJRUEXSXMBJLVWNYRTZJTHLEKDTWCGJ9OXDEZNWTKLTXXTKVFXDMRJUDAMDACRHJKZIJFJBZRVDLSTIMOWPTLLIVHCFUBMSQIPVPCSQAPZGHHNNQLWEHDTIQZQTAXJMTTROYOTIZSZKKQFXBHXFKSNAGZWHWEGXCKHSJQGBJGC9IKUSSJEAOHTPGPMIYPEXJXJKRLX9IOCUDKK9ONEUONYVGHRHXSAUZJXGVQNWSDZIYXXZMMJKRTOXSWJHBIMXPEMJTKIBSQKPICTKAQLJVOLZSGNVVBIOFJJKKJ9UBYKJWCBRPQLGBNEKEFRYCHB9PORAKEKTJZYZBFGZLONUJCPNINGUTYWTMDKTEPDASCHNVHXTUIOZ9PDALXKQYLILMUJEJWTYYAGEQXNNPLOMFGHSTFCNRMDFUSQREHFASDXZZOYNWNVKCRFZIDF9X9YKME9O9NJH9LFVNNSOXHSQOMBULFHBBPSGRCCGKWENQOZHRSIIHKKXAQTFISNVGIVMBDKSJYDW9VTZBAVYWWJAIYSCHGBIXKMKHBRTHWQMIWQVPWFUJQVUDZRDX9MMCXOQTQZGKECGIHATHUC9TDGKUKOYXEEBGTQYEO9KADOSMYWGQARTIUG9IUHQEBWSFHWDZCRCH9WDRJEUSQDBBGGKTOYBKYZX9LUKUGBVTEPXDOVVNSKPUWNARSILQNOQKATCUHMRVMKVHF9B9TEJDOMBDXLNKDJI9IIYRXNOQPBOVOEEGQFSZJJOFPNFVXOYZNLSOOA9FWDFJKDOQUHWHBNDWQZZVRBZJSLDVBRCGFCWXNUFTMLCHNXODSQMUIBPN9NMAXZXKUYYRUEDSLW9UEQYGSEBEOHGI9W9WAUFDEQCXVYOTBAAHFXHRGJBWGFZKIUMSWEXAD9EDALOOYQZIXKUOWJDHBHSEUDPJJRXANPPHUZWJM9KDVGJUAQHFJTWNFSULGSWMGMAOCAHTIYYJONOLKGNKRXWWZYWKITSFPQJNHQWAIJULYWPXQENPZTWZISXJBWYLENOINLFBRWWGWSWJONHIA99VGCSAT9XNMZUCSVEJMZJASUSXVUWFSVFXUIFWGIFKLEFLANHTITFCOPXRXTDXRDHLWP9RJHVQJMLPBFVHTIOQMSZECNPITTBTPLUHDQQZP9BWTWIZSOTMTZQWHCETRTKQPOHPJMJMLSBWC9ZRQAZDLAPJHAXRQZ9RUHHBPASWVUHBYX9FH9PLEHQEKCOMIHUNDVKOPKXEFM9CNP9LOLMBVHMMWNDCKHSYZEXOJAQOHMRXEGWMWFW9YVOZ9YDPHWVTKWFYUECSROYJENFPSIKHBJRSNWO9KQUENGPVULVYAQFAIYFSNIYR9LLRMYNCNQQAOPFNMWFSWSPMWNNIRNVKDZWRLGMPCBOVIMMXEZUFYPFPIGWTGOLDAEBZQADSOGVDZXHEWZGHNAAHMFPSOZD9SEPNCW9GTN9WLFDDCKMMXPXAEUUJPJFKKWMGUKVMYBH9AIEFAIIDJOAWRDKECA99XOYRSFZQKTVRJMTUAQJZUTKGXROESUYYSHTLRIZRPSNDFEEZWXCQONZYCD9TOHCBP9ISXQ9YSRZJ999Z9TETYOINSDGRBQSDTVFABHQNLTWGFYLHBHPVKLIBUMLVSURAOS9QHXDTIPKOJDLYOKRCEKCBMKVYIAKVA9WTGDWHIRUAWOVRKOSYTNIZAZNTJRFJDMNLGHTDKPKZDLBPQXRIRIVREMOBCPHMBBAUKNXHU9XIZNG9GD9LDIBBFPSI9PJNRCHXHNWAZXIACE9LUBNUWOK9LGJ9MKZQRI9CBCJUNALQKKVGGSPRJFAGCXNFO99YMLMKI9NVUZCZ9BCUEBSGMAVNKGWYWWQPZISMKAROXLQWEHOJIJOIIYRUDBNHRD9DEDQWQONAXKKSYMYCFTITZFKIXKZCGAVAFQIYEMESOIMWUUDSXJRR9RVWTAAHCOA9SCQBF9LAGPPYDXPEBKLHZ9KHKTXFP9XOVMVWIXEWMOISJHMQEXMYMZCUGEQNKGUNVRPUDPRX9IR9LBASIARWNFXXESPITSLYAQMLCLVTLHW9999999999999999999999999999999999999999999999999999FBIEUWD99A99999999C99999999DEXRPLKGBROUQMKCLMRPG9HFKCACDZ9AB9HOJQWERTYWERJNOYLW9PKLOGDUPC9DLGSUH9UHSKJOASJRU9MMRRSLICRITOROFC9FBVWLFEDNN9KJKYHUMRCJEUDGCYCWTBP9HHBEEJRFAU9FALRJWTU99NZK999999UE9VSBDVSRNTBZWPXYZPGAUTSWFLARLPXMHYBSTEUWIDOFJQJMVIACGUPTOMBWQO9AEADCFCMFJ999999WQKHJEXIHMOKQETOUTEO9JUPCDNAJQYZVXQRCXGYGEBOTMHE9HSJXVYVQUS9FPDLQWWKSYVDPCXX9LLAT"
	tx, err := NewTransaction(trytes)
	if err != nil {
		t.Fatal(err)
	}
	if !tx.HasValidNonce(18) {
		t.Error("cannot validate nonce")
	}
}

I don't understand the validation logic in the HasValidNonce function , looks nothing relate to the nonce, but only care about if the tx hash ends with 9

//HasValidNonce checks t's hash has valid MinWeightMagnitude.
func (t *Transaction) HasValidNonce(mwm int64) bool {
	h := t.Hash()
	for i := len(h) - 1; i > len(h)-1-int(mwm)/3; i-- {
		if h[i] != '9' {
			return false
		}
	}
	return true
}
@jeffwillette
Copy link
Contributor

jeffwillette commented Jan 7, 2018

I am looking into fixing that too. It is caused by the Hash function. It was introduced in 5ef65e7 and if I change the numberOfRounds in curl.go back to 27 (the value before this commit), it passes.

This is my first time looking into this code so I am unsure about how everything is working and I haven't been able to come up with a fix so far...

@jeffwillette
Copy link
Contributor

Reading over the spec and the javascript implementation and tests (https://github.com/iotaledger/iota.lib.js/blob/433f5d37a684abf875aaa6f1fb97082ab06218f0/lib/utils/inputValidator.js#L94) I think this is just a mistake in the test...

It looks like Curl-P-27 ends with 6 9's, but Curl-P-81 just needs to be 81 valid trytes...

@knarz
Copy link
Contributor

knarz commented Jan 7, 2018

This should be fixed with the latest commit.

We need to count the trailing zeros of the transaction hash and have >= mwm zeroes at the end.

Please re-open if there are any more problems.

@knarz knarz closed this as completed Jan 7, 2018
@jeffwillette
Copy link
Contributor

@knarz it looks like the problem was the input trytes into the test?

I am trying to understand the code here, so if you have time for a short explanation of why the input trytes were wrong, I would appreciate it!

@knarz
Copy link
Contributor

knarz commented Jan 8, 2018

The test used an old magnitude (18), so I changed it to use the DefaultMinWeightMagnitude constant, and the trytes were for another mwm.
Also the HasValidNonce was counting trailing 9 trytes, which works fine for any mwm mod 3 == 0 but would fail for the current mwm of 14.

I think that's everything.

luca-moser added a commit that referenced this issue Jan 30, 2019
# This is the 1st commit message:

adds account base impl.

# This is the commit message #2:

fixes SendTrytes() returning reversed bundle

let DoPoW() return trytes in bundle order

fixes AttachToTangle reversing already reversed trytes

# This is the commit message #3:

adds base implementation

# This is the commit message #4:

adds some todos

# This is the commit message #5:

makes addPendingTransfer also store the spent address indices, removes action channels by only using one, adds multi send, removes LastDepositAddress()

# This is the commit message #6:

moves account package

# This is the commit message #7:

adds clean shutdown, precomputed next dep. address, errors channel for internal errors

# This is the commit message #8:

changes state schema, adds promotion/reattachment

# This is the commit message #9:

smh

# This is the commit message #10:

adds deposit requests/conditions

# This is the commit message #11:

fixes tests

# This is the commit message #12:

split packages, make badger use gob instead of json

# This is the commit message #13:

removes SendMulti, makes Send take a variadic parameter

# This is the commit message #14:

fixes tests, reduces to single Send function, splits top level account files

# This is the commit message #15:

implements quorum based querying of nodes

# This is the commit message #16:

switch to xxhash for res body, make primary node optional

# This is the commit message #17:

moves generating hash before lock

# This is the commit message #18:

adds no response tolerance to quorum settings

# This is the commit message #19:

subtract error count from total votes

# This is the commit message #20:

fixes no-response tolerance, adds more descriptive error message

# This is the commit message #21:

adds override for commands to execute in quorum

# This is the commit message #22:

adds quorumable latest subtangle milestone query

# This is the commit message #23:

adds customizable defaults for certain calls where no quorum was reached

# This is the commit message #24:

simplify injectDefault

# This is the commit message #25:

adds partial test

# This is the commit message #26:

fixes some typos, adds additional test for explicit error responses

# This is the commit message #27:

adds UsableBalance()/TotalBalance()

# This is the commit message #28:

refactor input selection, wrap errors with more descriptive messages

# This is the commit message #29:

adds specific checks when querying the latest solid subtangle milestone

# This is the commit message #30:

fixes typo

# This is the commit message #31:

adds safe component update to account, adds special treatment for quorum findTransactions call

# This is the commit message #32:

fixes non receive events to be not fired until first poll went through

# This is the commit message #33:

modulerizes events out of the account object, adds builder pattern for creating accounts and event listeners

# This is the commit message #34:

changes store interface to return maps, optimizes pertail receive event filter, optimizes event listener registration

# This is the commit message #35:

adds correct addr gen. cancellation,fixes receive events being dropped, fixes duplicated receive events for messages
luca-moser added a commit that referenced this issue Jan 30, 2019
# This is the 1st commit message:

adds account base impl.

# This is the commit message #2:

fixes SendTrytes() returning reversed bundle

let DoPoW() return trytes in bundle order

fixes AttachToTangle reversing already reversed trytes

# This is the commit message #3:

adds base implementation

# This is the commit message #4:

adds some todos

# This is the commit message #5:

makes addPendingTransfer also store the spent address indices, removes action channels by only using one, adds multi send, removes LastDepositAddress()

# This is the commit message #6:

moves account package

# This is the commit message #7:

adds clean shutdown, precomputed next dep. address, errors channel for internal errors

# This is the commit message #8:

changes state schema, adds promotion/reattachment

# This is the commit message #9:

smh

# This is the commit message #10:

adds deposit requests/conditions

# This is the commit message #11:

fixes tests

# This is the commit message #12:

split packages, make badger use gob instead of json

# This is the commit message #13:

removes SendMulti, makes Send take a variadic parameter

# This is the commit message #14:

fixes tests, reduces to single Send function, splits top level account files

# This is the commit message #15:

implements quorum based querying of nodes

# This is the commit message #16:

switch to xxhash for res body, make primary node optional

# This is the commit message #17:

moves generating hash before lock

# This is the commit message #18:

adds no response tolerance to quorum settings

# This is the commit message #19:

subtract error count from total votes

# This is the commit message #20:

fixes no-response tolerance, adds more descriptive error message

# This is the commit message #21:

adds override for commands to execute in quorum

# This is the commit message #22:

adds quorumable latest subtangle milestone query

# This is the commit message #23:

adds customizable defaults for certain calls where no quorum was reached

# This is the commit message #24:

simplify injectDefault

# This is the commit message #25:

adds partial test

# This is the commit message #26:

fixes some typos, adds additional test for explicit error responses

# This is the commit message #27:

adds UsableBalance()/TotalBalance()

# This is the commit message #28:

refactor input selection, wrap errors with more descriptive messages

# This is the commit message #29:

adds specific checks when querying the latest solid subtangle milestone

# This is the commit message #30:

fixes typo

# This is the commit message #31:

adds safe component update to account, adds special treatment for quorum findTransactions call

# This is the commit message #32:

fixes non receive events to be not fired until first poll went through

# This is the commit message #33:

modulerizes events out of the account object, adds builder pattern for creating accounts and event listeners

# This is the commit message #34:

changes store interface to return maps, optimizes pertail receive event filter, optimizes event listener registration

# This is the commit message #35:

adds correct addr gen. cancellation,fixes receive events being dropped, fixes duplicated receive events for messages

# This is the commit message #36:

adds the concepts of plugins, makes promoter and transfer polling plugins, simplfies event machine

# This is the commit message #37:

store signatures too

# This is the commit message #38:

only store non empty signatures

# This is the commit message #39:

resolves conflicts of cherry pick

# This is the commit message #40:

dont generete addr. checksum at unnedded places, omit empty CDR fields in marshaling

# This is the commit message #41:

fixes checkConsistency for quorum, optimizes input selection, replaces event loop with RW lock, fixes promoter not having a time source, fixes transfer poller not shutting down, fixes tests, renames input selection func., optimizes request parsing of IRI calls

# This is the commit message #42:

changes pending transfer data to trytes, fixes reattacher creating inconsistent bundles, fixes primary CDRs with 0 balance being removed

# This is the commit message #43:

adjusts to latest spec

# This is the commit message #44:

adjusts json field tag for CDRs, fixes typo in plugins

# This is the commit message #45:

adds oracle package

# This is the commit message #46:

moves store impls. into own pkgs

# This is the commit message #47:

fixes gocks and removes quorum code from branch
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

No branches or pull requests

3 participants