Change TraceNumber and OriginalTrace to strings to prevent errors

[adamdecaf]

(The original issue was #236.)

TraceNumber (and likely other fields) have known edge cases and bugs.

• TraceNumber is a 15 digit value whose prefix is an ABA routing number.
  • These ABA routing numbers often have a leading zero.
• int in Go is only defined: "int is a signed integer type that is at least 32 bits in size"

We should change this type to be a string (perhaps a TraceNumber type) to prevent both bugs. This is a breaking change however, so we should release it sooner rather than later.

We should also scan the other fields (int types) for similar problems.

[adamdecaf]

For reference, here's all other int types aside from TraceNumber and OriginalTrace:

	AddendaRecordIndicator             int // 0 or 1
	AddendaRecords                     int // count 
	Amount                             int // non-negative int
	BatchCount                         int // count
	BatchNumber                        int // count 
	BlockCount                         int // count 
	EntryAddendaCount                  int // count 
	EntryDetailSequenceNumber          int // non-negative int
	EntryHash                          int // sum of ABA's 
	ForeignExchangeReferenceIndicator  int // fixed set of values (0, 1, 2)
	ForeignPaymentAmount               int // non-negative int
	OriginatorStatusCode               int // fixed set of values (0, 1, 2)
	SequenceNumber                     int // non-negative int
	ServiceClassCode                   int // fixed set of values
	TotalCreditEntryDollarAmountInFile int // sum
	TotalDebitEntryDollarAmountInFile  int // sum 
	TransactionCode                    int // fixed set of values

[adamdecaf]

Going by that table I don't think we need to change any other values. Double checking with @bkmoovio and @wadearnold

[bkmoovio]

Remind me why we are doing this again? If I recall you were running a 32 bit system? Didn’t you put a check in that says we don’t support 32 bit systems? Just trying to remember the issue?

[adamdecaf]

Yes, but the 32bit issue was a problem of overflow.

There's still the problem of any integer type truncating leading 0's from routing numbers.

[bkmoovio]

Which routing numbers are not string?

[adamdecaf]

TraceNumber (and OriginalTrace) have routing numbers as their prefix.

[bkmoovio]

Why hasn't it been an issue? Sorry just trying to understand that? I'm just wondering if you are fixing something that may never be an issue? Or is it that we just have had a routing number with leading 0?

[bkmoovio]

I just changed the PPD file write to have routing numbers with leading zero's. There were no errors. Apologies. I'm sure there is an issue, but I'm just not seeing it.

101 031380104 0210428821811090000A094101Federal Reserve Bank My Bank Name
5220Name on Account 021042882 PPDREG.SALARY 181110 0121042880000001
62223138010412345678 0100000000 Receiver Account Name 0121042880000001
82200000010023138010000000000000000100000000021042882 121042880000001
9000001000001000000010023138010000000000000000100000000
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999
9999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999

[adamdecaf]

I see a few problems, but maybe we can suggest TraceNumberField().

The following are all invalid, but could be written by accident.

• string(ed.TraceNumber)[:9] will often panic (ed.TraceNumber is converted to a UTF-8 codepoint)
• fmt.Sprintf("%d", ed.TraceNumber)[:9] is also invalid for leading-zero routing numbers. (The 0 is missing.)

[bkmoovio]

The underlying library uses TraceNumberField(), right?

[bkmoovio]

// TraceNumberField returns a zero padded TraceNumber string
func (ed *EntryDetail) TraceNumberField() string {
return ed.numericField(ed.TraceNumber, 15)
}

[adamdecaf]

I was thinking about this over the weekend. What if we un-export TraceNumber and use the unexported value to store what's in a file we read, but otherwise concat the two values which comprise it?

   traceNumber string
// ... later on

// returns either traceNumber (if non-empty) or concat the strings
func (..) TraceNumberField() string {

}

Doing so breaks an implicit 1:1 mapping we have from "ACH field" to "struct field".

[wadearnold]

Consumers of the API will need access to the trace number. Without question, they will need to be able to read it. For Returns they will need to be able to set the trace number as it has to be the original number. Currently a Return, Dishonored Return, Contested Dishonored Return all use the same entry detail struct.

[adamdecaf]

Gah, I forgot to imply a setter would exist. func (..) SetTraceNumber(str string) or something.

[adamdecaf]

Doing so breaks an implicit 1:1 mapping we have from "ACH field" to "struct field".

Is this something we're ok with changing?

[bkmoovio]

#378