Make basics.Address include checksums in its String and MarshalText methods about go-algorand HOT 4 CLOSED

basics.ChecksumAddress doesn't need to exist; the checksumming functionality can be brought into the MarshalText / UnmarshalText / String methods of basics.Address. This should allow most of the code in inspect.go to be deleted.

For context, whenever we represent an address as text, we compute and append a checksum -- this lets us catch errors if someone mistypes an address. When using addresses internally we don't bother keeping around the checksum; for example, when sending a transaction over the wire, we don't include the checksum in the binary (msgpack) representation of the sender address. The checksum is cheap enough that it's fine to recompute it if we ever need to display an address to a user.
At one point we thought it would be a good idea to have separate types for the internal / external representations of addresses -- basics.Address and basics.ChecksumAddress. But this turns out to add boilerplate (have to cast to a ChecksumAddress whenever displaying address to user) and make it easy to accidentally display to the user a non-checksummed address (by forgetting the cast).

Whenever we're representing an address as text (and only when we're representing an address as text), we want a checksum. So there don't need to be two separate types -- calculating the checksum should just happen inside the MarshalText, UnmarshalText, and String methods of basics.Address. Then basics.ChecksumAddress can be removed and all uses of basics.ChecksumAddress in our codebase can be replaced with basics.Address.

inspect.go just makes types identical to other types defined elsewhere but with checksumAddress where other types have Address so that e.g., the sender address of a transaction will print with the checksum. If basics.Address had a String method that added the checksum then fmt.Printf("%v", transaction) would already include checksums and most of the boilerplate in inspect.go could be removed.

(As a sidenote, I think the multisig-related code in crypto.go might still require these boilerplate wrappers because multisigs in some places use crypto.PublicKey instead of basics.Address. Multisig internals definitely could use some cleaning-up, but that's out of scope of this particular issue.)

from go-algorand.

(not sure crypto.PublicKey is an issue - it's a fixed array? Can you elaborate)

from go-algorand.

crypto.PublicKey doesn't (and probably shoudln't) include base32 or checksum printing code.

Multisigs include crypto.PublicKeys instead of basics.Addresses because (among other reasons) the basics package imports the crypto package and so the crypto package can't import the basics package. Changing that is probably out of scope of this issue.

So inspect.go will probably still need to contain code to pretty-print multisigs. However, we should be able to get rid of the non-multisig boilerplate like inspectTransaction, inspectTxnHeader, inspectPaymentTxnFields, etc.

from go-algorand.

Fixed by PR #102 (thanks zacharyestep!)

from go-algorand.